| Message ID | 457056cbc6dcc00958b1f4e0cad35121e0cd1557.1657121519.git-series.marmarek@invisiblethingslab.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add Xue - console over USB 3 Debug Capability | expand |
On 06.07.2022 17:32, Marek Marczykowski-Górecki wrote: > --- a/xen/drivers/passthrough/amd/iommu_acpi.c > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c > @@ -1078,6 +1078,20 @@ static inline bool_t is_ivmd_block(u8 type) > type == ACPI_IVRS_TYPE_MEMORY_IOMMU); > } > > +static int __init cf_check add_one_extra_ivmd(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt) > +{ > + struct acpi_ivrs_memory ivmd; > + > + ivmd.start_address = start << PAGE_SHIFT; > + ivmd.memory_length = nr << PAGE_SHIFT; Aren't these at risk of truncation on 32-bit architectures? We have suitable wrappers for such conversions, avoiding such issues. > + ivmd.header.flags = ACPI_IVMD_UNITY | > + ACPI_IVMD_READ | ACPI_IVMD_WRITE; > + ivmd.header.length = sizeof(ivmd); > + ivmd.header.device_id = id; > + ivmd.header.type = ACPI_IVRS_TYPE_MEMORY_ONE; Please make these the variable's initializer. Jan
On Thu, Jul 14, 2022 at 12:22:36PM +0200, Jan Beulich wrote: > On 06.07.2022 17:32, Marek Marczykowski-Górecki wrote: > > --- a/xen/drivers/passthrough/amd/iommu_acpi.c > > +++ b/xen/drivers/passthrough/amd/iommu_acpi.c > > @@ -1078,6 +1078,20 @@ static inline bool_t is_ivmd_block(u8 type) > > type == ACPI_IVRS_TYPE_MEMORY_IOMMU); > > } > > > > +static int __init cf_check add_one_extra_ivmd(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt) > > +{ > > + struct acpi_ivrs_memory ivmd; > > + > > + ivmd.start_address = start << PAGE_SHIFT; > > + ivmd.memory_length = nr << PAGE_SHIFT; > > Aren't these at risk of truncation on 32-bit architectures? We have > suitable wrappers for such conversions, avoiding such issues. Well, this (and the vtd equivalent) is x86-only, so it's always 64-bit. Anyway, what are those wrappers? > > + ivmd.header.flags = ACPI_IVMD_UNITY | > > + ACPI_IVMD_READ | ACPI_IVMD_WRITE; > > + ivmd.header.length = sizeof(ivmd); > > + ivmd.header.device_id = id; > > + ivmd.header.type = ACPI_IVRS_TYPE_MEMORY_ONE; > > Please make these the variable's initializer. > > Jan
On 18.07.2022 13:35, Marek Marczykowski-Górecki wrote: > On Thu, Jul 14, 2022 at 12:22:36PM +0200, Jan Beulich wrote: >> On 06.07.2022 17:32, Marek Marczykowski-Górecki wrote: >>> --- a/xen/drivers/passthrough/amd/iommu_acpi.c >>> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c >>> @@ -1078,6 +1078,20 @@ static inline bool_t is_ivmd_block(u8 type) >>> type == ACPI_IVRS_TYPE_MEMORY_IOMMU); >>> } >>> >>> +static int __init cf_check add_one_extra_ivmd(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt) >>> +{ >>> + struct acpi_ivrs_memory ivmd; >>> + >>> + ivmd.start_address = start << PAGE_SHIFT; >>> + ivmd.memory_length = nr << PAGE_SHIFT; >> >> Aren't these at risk of truncation on 32-bit architectures? We have >> suitable wrappers for such conversions, avoiding such issues. > > Well, this (and the vtd equivalent) is x86-only, so it's always 64-bit. Nevertheless writing things cleanly everywhere will reduce the risk of somebody cloning this code on the assumption that it's correct. > Anyway, what are those wrappers? pfn_to_paddr() Jan
diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c index ac6835225bae..2a4896c05442 100644 --- a/xen/drivers/passthrough/amd/iommu_acpi.c +++ b/xen/drivers/passthrough/amd/iommu_acpi.c @@ -1078,6 +1078,20 @@ static inline bool_t is_ivmd_block(u8 type) type == ACPI_IVRS_TYPE_MEMORY_IOMMU); } +static int __init cf_check add_one_extra_ivmd(xen_pfn_t start, xen_ulong_t nr, u32 id, void *ctxt) +{ + struct acpi_ivrs_memory ivmd; + + ivmd.start_address = start << PAGE_SHIFT; + ivmd.memory_length = nr << PAGE_SHIFT; + ivmd.header.flags = ACPI_IVMD_UNITY | + ACPI_IVMD_READ | ACPI_IVMD_WRITE; + ivmd.header.length = sizeof(ivmd); + ivmd.header.device_id = id; + ivmd.header.type = ACPI_IVRS_TYPE_MEMORY_ONE; + return parse_ivmd_block(&ivmd); +} + static int __init cf_check parse_ivrs_table(struct acpi_table_header *table) { const struct acpi_ivrs_header *ivrs_block; @@ -1121,6 +1135,8 @@ static int __init cf_check parse_ivrs_table(struct acpi_table_header *table) AMD_IOMMU_DEBUG("IVMD: %u command line provided entries\n", nr_ivmd); for ( i = 0; !error && i < nr_ivmd; ++i ) error = parse_ivmd_block(user_ivmds + i); + if ( !error ) + error = iommu_get_extra_reserved_device_memory(add_one_extra_ivmd, NULL); /* Each IO-APIC must have been mentioned in the table. */ for ( apic = 0; !error && iommu_intremap && apic < nr_ioapics; ++apic )
Register common device reserved memory similar to how ivmd= parameter is handled. Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com> --- xen/drivers/passthrough/amd/iommu_acpi.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)