| Message ID | 5D024E170200007800237DFD@prv1-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: AMD x2APIC support | expand |
On Thu, Jun 13, 2019 at 07:22:31AM -0600, Jan Beulich wrote: > This also takes care of several of the shift values wrongly having been > specified as hex rather than dec. > > Take the opportunity and add further fields. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/drivers/passthrough/amd/iommu_detect.c > +++ b/xen/drivers/passthrough/amd/iommu_detect.c > @@ -60,43 +60,72 @@ static int __init get_iommu_capabilities > > void __init get_iommu_features(struct amd_iommu *iommu) > { > - u32 low, high; > - int i = 0 ; > - static const char *__initdata feature_str[] = { > - "- Prefetch Pages Command", > - "- Peripheral Page Service Request", > - "- X2APIC Supported", > - "- NX bit Supported", > - "- Guest Translation", > - "- Reserved bit [5]", > - "- Invalidate All Command", > - "- Guest APIC supported", > - "- Hardware Error Registers", > - "- Performance Counters", > - NULL > - }; > - > ASSERT( iommu->mmio_base ); > > if ( !iommu_has_cap(iommu, PCI_CAP_EFRSUP_SHIFT) ) > { > - iommu->features = 0; > + iommu->features.raw = 0; > return; > } > > - low = readl(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET); > - high = readl(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET + 4); > - > - iommu->features = ((u64)high << 32) | low; > + iommu->features.raw = > + readq(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET); > > printk("AMD-Vi: IOMMU Extended Features:\n"); > > - while ( feature_str[i] ) > +#define MASK(fld) ((union amd_iommu_ext_features){ .flds.fld = ~0 }).raw > +#define FEAT(fld, str) do { \ > + if ( MASK(fld) & (MASK(fld) - 1) ) \ > + printk( "- " str ": %#x\n", iommu->features.flds.fld); \ > + else if ( iommu->features.raw & MASK(fld) ) \ > + printk( "- " str "\n"); \ > +} while ( false ) > + > + FEAT(pref_sup, "Prefetch Pages Command"); > + FEAT(ppr_sup, "Peripheral Page Service Request"); > + FEAT(xt_sup, "x2APIC"); > + FEAT(nx_sup, "NX bit"); > + FEAT(gappi_sup, "Guest APIC Physical Processor Interrupt"); > + FEAT(ia_sup, "Invalidate All Command"); > + FEAT(ga_sup, "Guest APIC"); > + FEAT(he_sup, "Hardware Error Registers"); > + FEAT(pc_sup, "Performance Counters"); > + FEAT(hats, "Host Address Translation Size"); > + > + if ( iommu->features.flds.gt_sup ) > { > - if ( amd_iommu_has_feature(iommu, i) ) > - printk( " %s\n", feature_str[i]); > - i++; > + FEAT(gats, "Guest Address Translation Size"); > + FEAT(glx_sup, "Guest CR3 Root Table Level"); > + FEAT(pas_max, "Maximum PASID"); > } > + > + FEAT(smif_sup, "SMI Filter Register"); > + FEAT(smif_rc, "SMI Filter Register Count"); > + FEAT(gam_sup, "Guest Virtual APIC Modes"); > + FEAT(dual_ppr_log_sup, "Dual PPR Log"); > + FEAT(dual_event_log_sup, "Dual Event Log"); > + FEAT(sat_sup, "Secure ATS"); > + FEAT(us_sup, "User / Supervisor Page Protection"); > + FEAT(dev_tbl_seg_sup, "Device Table Segmentation"); > + FEAT(ppr_early_of_sup, "PPR Log Overflow Early Warning"); > + FEAT(ppr_auto_rsp_sup, "PPR Automatic Response"); > + FEAT(marc_sup, "Memory Access Routing and Control"); > + FEAT(blk_stop_mrk_sup, "Block StopMark Message"); > + FEAT(perf_opt_sup , "Performance Optimization"); > + FEAT(msi_cap_mmio_sup, "MSI Capability MMIO Access"); > + FEAT(gio_sup, "Guest I/O Protection"); > + FEAT(ha_sup, "Host Access"); > + FEAT(eph_sup, "Enhanced PPR Handling"); > + FEAT(attr_fw_sup, "Attribute Forward"); > + FEAT(hd_sup, "Host Dirty"); > + FEAT(inv_iotlb_type_sup, "Invalidate IOTLB Type"); > + FEAT(viommu_sup, "Virtualized IOMMU"); > + FEAT(vm_guard_io_sup, "VMGuard I/O Support"); > + FEAT(vm_table_size, "VM Table Size"); > + FEAT(ga_update_dis_sup, "Guest Access Bit Update Disable"); > + > +#undef FEAT > +#undef MASK > } > > int __init amd_iommu_detect_one_acpi( > --- a/xen/drivers/passthrough/amd/iommu_guest.c > +++ b/xen/drivers/passthrough/amd/iommu_guest.c > @@ -638,7 +638,7 @@ static uint64_t iommu_mmio_read64(struct > val = reg_to_u64(iommu->reg_status); > break; > case IOMMU_EXT_FEATURE_MMIO_OFFSET: > - val = reg_to_u64(iommu->reg_ext_feature); > + val = iommu->reg_ext_feature.raw; > break; > > default: > @@ -802,39 +802,26 @@ int guest_iommu_set_base(struct domain * > /* Initialize mmio read only bits */ > static void guest_iommu_reg_init(struct guest_iommu *iommu) > { > - uint32_t lower, upper; > + union amd_iommu_ext_features ef = { > + /* Support prefetch */ > + .flds.pref_sup = 1, > + /* Support PPR log */ > + .flds.ppr_sup = 1, > + /* Support guest translation */ > + .flds.gt_sup = 1, > + /* Support invalidate all command */ > + .flds.ia_sup = 1, > + /* Host translation size has 6 levels */ > + .flds.hats = HOST_ADDRESS_SIZE_6_LEVEL, > + /* Guest translation size has 6 levels */ > + .flds.gats = GUEST_ADDRESS_SIZE_6_LEVEL, > + /* Single level gCR3 */ > + .flds.glx_sup = GUEST_CR3_1_LEVEL, > + /* 9 bit PASID */ > + .flds.pas_max = PASMAX_9_bit, > + }; > > - lower = upper = 0; > - /* Support prefetch */ > - iommu_set_bit(&lower,IOMMU_EXT_FEATURE_PREFSUP_SHIFT); > - /* Support PPR log */ > - iommu_set_bit(&lower,IOMMU_EXT_FEATURE_PPRSUP_SHIFT); > - /* Support guest translation */ > - iommu_set_bit(&lower,IOMMU_EXT_FEATURE_GTSUP_SHIFT); > - /* Support invalidate all command */ > - iommu_set_bit(&lower,IOMMU_EXT_FEATURE_IASUP_SHIFT); > - > - /* Host translation size has 6 levels */ > - set_field_in_reg_u32(HOST_ADDRESS_SIZE_6_LEVEL, lower, > - IOMMU_EXT_FEATURE_HATS_MASK, > - IOMMU_EXT_FEATURE_HATS_SHIFT, > - &lower); > - /* Guest translation size has 6 levels */ > - set_field_in_reg_u32(GUEST_ADDRESS_SIZE_6_LEVEL, lower, > - IOMMU_EXT_FEATURE_GATS_MASK, > - IOMMU_EXT_FEATURE_GATS_SHIFT, > - &lower); > - /* Single level gCR3 */ > - set_field_in_reg_u32(GUEST_CR3_1_LEVEL, lower, > - IOMMU_EXT_FEATURE_GLXSUP_MASK, > - IOMMU_EXT_FEATURE_GLXSUP_SHIFT, &lower); > - /* 9 bit PASID */ > - set_field_in_reg_u32(PASMAX_9_bit, upper, > - IOMMU_EXT_FEATURE_PASMAX_MASK, > - IOMMU_EXT_FEATURE_PASMAX_SHIFT, &upper); > - > - iommu->reg_ext_feature.lo = lower; > - iommu->reg_ext_feature.hi = upper; > + iommu->reg_ext_feature = ef; > } > > static int guest_iommu_mmio_range(struct vcpu *v, unsigned long addr) > --- a/xen/drivers/passthrough/amd/iommu_init.c > +++ b/xen/drivers/passthrough/amd/iommu_init.c > @@ -883,7 +883,7 @@ static void enable_iommu(struct amd_iomm > register_iommu_event_log_in_mmio_space(iommu); > register_iommu_exclusion_range(iommu); > > - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_PPRSUP_SHIFT) ) > + if ( iommu->features.flds.ppr_sup ) > register_iommu_ppr_log_in_mmio_space(iommu); > > desc = irq_to_desc(iommu->msi.irq); > @@ -897,15 +897,15 @@ static void enable_iommu(struct amd_iomm > set_iommu_command_buffer_control(iommu, IOMMU_CONTROL_ENABLED); > set_iommu_event_log_control(iommu, IOMMU_CONTROL_ENABLED); > > - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_PPRSUP_SHIFT) ) > + if ( iommu->features.flds.ppr_sup ) > set_iommu_ppr_log_control(iommu, IOMMU_CONTROL_ENABLED); > > - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_GTSUP_SHIFT) ) > + if ( iommu->features.flds.gt_sup ) > set_iommu_guest_translation_control(iommu, IOMMU_CONTROL_ENABLED); > > set_iommu_translation_control(iommu, IOMMU_CONTROL_ENABLED); > > - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_IASUP_SHIFT) ) > + if ( iommu->features.flds.ia_sup ) > amd_iommu_flush_all_caches(iommu); > > iommu->enabled = 1; > @@ -928,10 +928,10 @@ static void disable_iommu(struct amd_iom > set_iommu_command_buffer_control(iommu, IOMMU_CONTROL_DISABLED); > set_iommu_event_log_control(iommu, IOMMU_CONTROL_DISABLED); > > - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_PPRSUP_SHIFT) ) > + if ( iommu->features.flds.ppr_sup ) > set_iommu_ppr_log_control(iommu, IOMMU_CONTROL_DISABLED); > > - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_GTSUP_SHIFT) ) > + if ( iommu->features.flds.gt_sup ) > set_iommu_guest_translation_control(iommu, IOMMU_CONTROL_DISABLED); > > set_iommu_translation_control(iommu, IOMMU_CONTROL_DISABLED); > @@ -1027,7 +1027,7 @@ static int __init amd_iommu_init_one(str > > get_iommu_features(iommu); > > - if ( iommu->features ) > + if ( iommu->features.raw ) > iommuv2_enabled = 1; > > if ( allocate_cmd_buffer(iommu) == NULL ) > @@ -1036,9 +1036,8 @@ static int __init amd_iommu_init_one(str > if ( allocate_event_log(iommu) == NULL ) > goto error_out; > > - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_PPRSUP_SHIFT) ) > - if ( allocate_ppr_log(iommu) == NULL ) > - goto error_out; > + if ( iommu->features.flds.ppr_sup && !allocate_ppr_log(iommu) ) > + goto error_out; > > if ( !set_iommu_interrupt_handler(iommu) ) > goto error_out; > @@ -1389,7 +1388,7 @@ void amd_iommu_resume(void) > } > > /* flush all cache entries after iommu re-enabled */ > - if ( !amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_IASUP_SHIFT) ) > + if ( !iommu->features.flds.ia_sup ) > { > invalidate_all_devices(); > invalidate_all_domain_pages(); > --- a/xen/include/asm-x86/amd-iommu.h > +++ b/xen/include/asm-x86/amd-iommu.h > @@ -83,7 +83,7 @@ struct amd_iommu { > iommu_cap_t cap; > > u8 ht_flags; > - u64 features; > + union amd_iommu_ext_features features; > > void *mmio_base; > unsigned long mmio_base_phys; > @@ -174,7 +174,7 @@ struct guest_iommu { > /* MMIO regs */ > struct mmio_reg reg_ctrl; /* MMIO offset 0018h */ > struct mmio_reg reg_status; /* MMIO offset 2020h */ > - struct mmio_reg reg_ext_feature; /* MMIO offset 0030h */ > + union amd_iommu_ext_features reg_ext_feature; /* MMIO offset 0030h */ > > /* guest interrupt settings */ > struct guest_iommu_msi msi; > --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > @@ -346,26 +346,57 @@ struct amd_iommu_dte { > #define IOMMU_EXCLUSION_LIMIT_HIGH_MASK 0xFFFFFFFF > #define IOMMU_EXCLUSION_LIMIT_HIGH_SHIFT 0 > > -/* Extended Feature Register*/ > +/* Extended Feature Register */ > #define IOMMU_EXT_FEATURE_MMIO_OFFSET 0x30 > -#define IOMMU_EXT_FEATURE_PREFSUP_SHIFT 0x0 > -#define IOMMU_EXT_FEATURE_PPRSUP_SHIFT 0x1 > -#define IOMMU_EXT_FEATURE_XTSUP_SHIFT 0x2 > -#define IOMMU_EXT_FEATURE_NXSUP_SHIFT 0x3 > -#define IOMMU_EXT_FEATURE_GTSUP_SHIFT 0x4 > -#define IOMMU_EXT_FEATURE_IASUP_SHIFT 0x6 > -#define IOMMU_EXT_FEATURE_GASUP_SHIFT 0x7 > -#define IOMMU_EXT_FEATURE_HESUP_SHIFT 0x8 > -#define IOMMU_EXT_FEATURE_PCSUP_SHIFT 0x9 > -#define IOMMU_EXT_FEATURE_HATS_SHIFT 0x10 > -#define IOMMU_EXT_FEATURE_HATS_MASK 0x00000C00 > -#define IOMMU_EXT_FEATURE_GATS_SHIFT 0x12 > -#define IOMMU_EXT_FEATURE_GATS_MASK 0x00003000 > -#define IOMMU_EXT_FEATURE_GLXSUP_SHIFT 0x14 > -#define IOMMU_EXT_FEATURE_GLXSUP_MASK 0x0000C000 > > -#define IOMMU_EXT_FEATURE_PASMAX_SHIFT 0x0 > -#define IOMMU_EXT_FEATURE_PASMAX_MASK 0x0000001F > +union amd_iommu_ext_features { > + uint64_t raw; > + struct { > + unsigned int pref_sup:1; > + unsigned int ppr_sup:1; > + unsigned int xt_sup:1; > + unsigned int nx_sup:1; > + unsigned int gt_sup:1; > + unsigned int gappi_sup:1; > + unsigned int ia_sup:1; > + unsigned int ga_sup:1; > + unsigned int he_sup:1; > + unsigned int pc_sup:1; > + unsigned int hats:2; > + unsigned int gats:2; > + unsigned int glx_sup:2; > + unsigned int smif_sup:2; > + unsigned int smif_rc:3; > + unsigned int gam_sup:3; > + unsigned int dual_ppr_log_sup:2; > + unsigned int :2; > + unsigned int dual_event_log_sup:2; > + unsigned int sat_sup:1; > + unsigned int :1; I think these might be flipped. > + unsigned int pas_max:5; > + unsigned int us_sup:1; > + unsigned int dev_tbl_seg_sup:2; > + unsigned int ppr_early_of_sup:1; > + unsigned int ppr_auto_rsp_sup:1; > + unsigned int marc_sup:2; > + unsigned int blk_stop_mrk_sup:1; > + unsigned int perf_opt_sup:1; > + unsigned int msi_cap_mmio_sup:1; > + unsigned int :1; > + unsigned int gio_sup:1; > + unsigned int ha_sup:1; > + unsigned int eph_sup:1; > + unsigned int attr_fw_sup:1; > + unsigned int hd_sup:1; > + unsigned int :1; > + unsigned int inv_iotlb_type_sup:1; > + unsigned int viommu_sup:1; > + unsigned int vm_guard_io_sup:1; > + unsigned int vm_table_size:4; > + unsigned int ga_update_dis_sup:1; > + unsigned int :2; > + } flds; > +}; > > /* Status Register*/ > #define IOMMU_STATUS_MMIO_OFFSET 0x2020 > --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h > +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h > @@ -219,13 +219,6 @@ static inline int iommu_has_cap(struct a > return !!(iommu->cap.header & (1u << bit)); > } > > -static inline int amd_iommu_has_feature(struct amd_iommu *iommu, uint32_t bit) > -{ > - if ( !iommu_has_cap(iommu, PCI_CAP_EFRSUP_SHIFT) ) > - return 0; > - return !!(iommu->features & (1U << bit)); > -} > - > /* access tail or head pointer of ring buffer */ > static inline uint32_t iommu_get_rb_pointer(uint32_t reg) > { > > >
On 13/06/2019 14:22, Jan Beulich wrote: > This also takes care of several of the shift values wrongly having been > specified as hex rather than dec. > > Take the opportunity and add further fields. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- a/xen/drivers/passthrough/amd/iommu_detect.c > +++ b/xen/drivers/passthrough/amd/iommu_detect.c > @@ -60,43 +60,72 @@ static int __init get_iommu_capabilities > > void __init get_iommu_features(struct amd_iommu *iommu) > { > - u32 low, high; > - int i = 0 ; > - static const char *__initdata feature_str[] = { > - "- Prefetch Pages Command", > - "- Peripheral Page Service Request", > - "- X2APIC Supported", > - "- NX bit Supported", > - "- Guest Translation", > - "- Reserved bit [5]", > - "- Invalidate All Command", > - "- Guest APIC supported", > - "- Hardware Error Registers", > - "- Performance Counters", > - NULL > - }; > - > ASSERT( iommu->mmio_base ); > > if ( !iommu_has_cap(iommu, PCI_CAP_EFRSUP_SHIFT) ) > { > - iommu->features = 0; > + iommu->features.raw = 0; > return; > } > > - low = readl(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET); > - high = readl(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET + 4); > - > - iommu->features = ((u64)high << 32) | low; > + iommu->features.raw = > + readq(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET); > > printk("AMD-Vi: IOMMU Extended Features:\n"); > > - while ( feature_str[i] ) > +#define MASK(fld) ((union amd_iommu_ext_features){ .flds.fld = ~0 }).raw > +#define FEAT(fld, str) do { \ > + if ( MASK(fld) & (MASK(fld) - 1) ) \ > + printk( "- " str ": %#x\n", iommu->features.flds.fld); \ > + else if ( iommu->features.raw & MASK(fld) ) \ > + printk( "- " str "\n"); \ > +} while ( false ) > + > + FEAT(pref_sup, "Prefetch Pages Command"); > + FEAT(ppr_sup, "Peripheral Page Service Request"); > + FEAT(xt_sup, "x2APIC"); > + FEAT(nx_sup, "NX bit"); > + FEAT(gappi_sup, "Guest APIC Physical Processor Interrupt"); > + FEAT(ia_sup, "Invalidate All Command"); > + FEAT(ga_sup, "Guest APIC"); > + FEAT(he_sup, "Hardware Error Registers"); > + FEAT(pc_sup, "Performance Counters"); > + FEAT(hats, "Host Address Translation Size"); > + > + if ( iommu->features.flds.gt_sup ) > { > - if ( amd_iommu_has_feature(iommu, i) ) > - printk( " %s\n", feature_str[i]); > - i++; > + FEAT(gats, "Guest Address Translation Size"); > + FEAT(glx_sup, "Guest CR3 Root Table Level"); > + FEAT(pas_max, "Maximum PASID"); > } > + > + FEAT(smif_sup, "SMI Filter Register"); > + FEAT(smif_rc, "SMI Filter Register Count"); > + FEAT(gam_sup, "Guest Virtual APIC Modes"); > + FEAT(dual_ppr_log_sup, "Dual PPR Log"); > + FEAT(dual_event_log_sup, "Dual Event Log"); > + FEAT(sat_sup, "Secure ATS"); > + FEAT(us_sup, "User / Supervisor Page Protection"); > + FEAT(dev_tbl_seg_sup, "Device Table Segmentation"); > + FEAT(ppr_early_of_sup, "PPR Log Overflow Early Warning"); > + FEAT(ppr_auto_rsp_sup, "PPR Automatic Response"); > + FEAT(marc_sup, "Memory Access Routing and Control"); > + FEAT(blk_stop_mrk_sup, "Block StopMark Message"); > + FEAT(perf_opt_sup , "Performance Optimization"); > + FEAT(msi_cap_mmio_sup, "MSI Capability MMIO Access"); > + FEAT(gio_sup, "Guest I/O Protection"); > + FEAT(ha_sup, "Host Access"); > + FEAT(eph_sup, "Enhanced PPR Handling"); > + FEAT(attr_fw_sup, "Attribute Forward"); > + FEAT(hd_sup, "Host Dirty"); > + FEAT(inv_iotlb_type_sup, "Invalidate IOTLB Type"); > + FEAT(viommu_sup, "Virtualized IOMMU"); > + FEAT(vm_guard_io_sup, "VMGuard I/O Support"); > + FEAT(vm_table_size, "VM Table Size"); > + FEAT(ga_update_dis_sup, "Guest Access Bit Update Disable"); > + > +#undef FEAT > +#undef MASK > } So this is fine, but does come with a downside. This is the log from a Rome system: (XEN) [ 17.928225] AMD-Vi: IOMMU Extended Features: (XEN) [ 17.988264] - Peripheral Page Service Request (XEN) [ 18.048082] - x2APIC (XEN) [ 18.104735] - NX bit (XEN) [ 18.160751] - Invalidate All Command (XEN) [ 18.218116] - Guest APIC (XEN) [ 18.273660] - Performance Counters (XEN) [ 18.329868] - Host Address Translation Size: 0x2 (XEN) [ 18.387363] - Guest Address Translation Size: 0 (XEN) [ 18.444446] - Guest CR3 Root Table Level: 0x1 (XEN) [ 18.501006] - Maximum PASID: 0xf (XEN) [ 18.555753] - SMI Filter Register: 0x1 (XEN) [ 18.610773] - SMI Filter Register Count: 0x2 (XEN) [ 18.666116] - Guest Virtual APIC Modes: 0x1 (XEN) [ 18.721036] - Dual PPR Log: 0x2 (XEN) [ 18.774237] - Dual Event Log: 0x2 (XEN) [ 18.827164] - User / Supervisor Page Protection (XEN) [ 18.881374] - Device Table Segmentation: 0x3 (XEN) [ 18.934949] - PPR Log Overflow Early Warning (XEN) [ 18.988186] - PPR Automatic Response (XEN) [ 19.040193] - Memory Access Routing and Control: 0x1 (XEN) [ 19.093770] - Block StopMark Message (XEN) [ 19.145300] - Performance Optimization (XEN) [ 19.196603] - MSI Capability MMIO Access (XEN) [ 19.247754] - Guest I/O Protection (XEN) [ 19.297811] - Host Access (XEN) [ 19.346396] - Enhanced PPR Handling (XEN) [ 19.395647] - Attribute Forward (XEN) [ 19.443986] - Virtualized IOMMU (XEN) [ 19.491828] - VMGuard I/O Support (XEN) [ 19.539404] - VM Table Size: 0x2 (XEN) [ 19.589837] AMD-Vi: IOMMU Extended Features: (XEN) [ 19.637829] - Peripheral Page Service Request (XEN) [ 19.685607] - x2APIC (XEN) [ 19.730231] - NX bit (XEN) [ 19.774195] - Invalidate All Command (XEN) [ 19.819528] - Guest APIC (XEN) [ 19.863027] - Performance Counters (XEN) [ 19.907165] - Host Address Translation Size: 0x2 (XEN) [ 19.952578] - Guest Address Translation Size: 0 (XEN) [ 19.997588] - Guest CR3 Root Table Level: 0x1 (XEN) [ 20.042057] - Maximum PASID: 0xf (XEN) [ 20.084732] - SMI Filter Register: 0x1 (XEN) [ 20.127648] - SMI Filter Register Count: 0x2 (XEN) [ 20.170910] - Guest Virtual APIC Modes: 0x1 (XEN) [ 20.213702] - Dual PPR Log: 0x2 (XEN) [ 20.254765] - Dual Event Log: 0x2 (XEN) [ 20.295567] - User / Supervisor Page Protection (XEN) [ 20.337626] - Device Table Segmentation: 0x3 (XEN) [ 20.379042] - PPR Log Overflow Early Warning (XEN) [ 20.420094] - PPR Automatic Response (XEN) [ 20.459890] - Memory Access Routing and Control: 0x1 (XEN) [ 20.501248] - Block StopMark Message (XEN) [ 20.540577] - Performance Optimization (XEN) [ 20.579876] - MSI Capability MMIO Access (XEN) [ 20.619309] - Guest I/O Protection (XEN) [ 20.658041] - Host Access (XEN) [ 20.695937] - Enhanced PPR Handling (XEN) [ 20.735276] - Attribute Forward (XEN) [ 20.773830] - Virtualized IOMMU (XEN) [ 20.812335] - VMGuard I/O Support (XEN) [ 20.850868] - VM Table Size: 0x2 (XEN) [ 20.892543] AMD-Vi: IOMMU Extended Features: (XEN) [ 20.932087] - Peripheral Page Service Request (XEN) [ 20.971756] - x2APIC (XEN) [ 21.008681] - NX bit (XEN) [ 21.045279] - Invalidate All Command (XEN) [ 21.083567] - Guest APIC (XEN) [ 21.120410] - Performance Counters (XEN) [ 21.158386] - Host Address Translation Size: 0x2 (XEN) [ 21.197956] - Guest Address Translation Size: 0 (XEN) [ 21.237433] - Guest CR3 Root Table Level: 0x1 (XEN) [ 21.276709] - Maximum PASID: 0xf (XEN) [ 21.314538] - SMI Filter Register: 0x1 (XEN) [ 21.352844] - SMI Filter Register Count: 0x2 (XEN) [ 21.391728] - Guest Virtual APIC Modes: 0x1 (XEN) [ 21.430614] - Dual PPR Log: 0x2 (XEN) [ 21.468150] - Dual Event Log: 0x2 (XEN) [ 21.505833] - User / Supervisor Page Protection (XEN) [ 21.545270] - Device Table Segmentation: 0x3 (XEN) [ 21.584538] - PPR Log Overflow Early Warning (XEN) [ 21.623928] - PPR Automatic Response (XEN) [ 21.662564] - Memory Access Routing and Control: 0x1 (XEN) [ 21.703268] - Block StopMark Message (XEN) [ 21.742415] - Performance Optimization (XEN) [ 21.781692] - MSI Capability MMIO Access (XEN) [ 21.821159] - Guest I/O Protection (XEN) [ 21.859916] - Host Access (XEN) [ 21.897811] - Enhanced PPR Handling (XEN) [ 21.936818] - Attribute Forward (XEN) [ 21.975387] - Virtualized IOMMU (XEN) [ 22.013898] - VMGuard I/O Support (XEN) [ 22.052448] - VM Table Size: 0x2 (XEN) [ 22.094140] AMD-Vi: IOMMU Extended Features: (XEN) [ 22.133685] - Peripheral Page Service Request (XEN) [ 22.173363] - x2APIC (XEN) [ 22.210287] - NX bit (XEN) [ 22.246888] - Invalidate All Command (XEN) [ 22.285176] - Guest APIC (XEN) [ 22.322008] - Performance Counters (XEN) [ 22.359994] - Host Address Translation Size: 0x2 (XEN) [ 22.399552] - Guest Address Translation Size: 0 (XEN) [ 22.439028] - Guest CR3 Root Table Level: 0x1 (XEN) [ 22.478307] - Maximum PASID: 0xf (XEN) [ 22.516133] - SMI Filter Register: 0x1 (XEN) [ 22.554441] - SMI Filter Register Count: 0x2 (XEN) [ 22.593345] - Guest Virtual APIC Modes: 0x1 (XEN) [ 22.632221] - Dual PPR Log: 0x2 (XEN) [ 22.669766] - Dual Event Log: 0x2 (XEN) [ 22.707455] - User / Supervisor Page Protection (XEN) [ 22.746896] - Device Table Segmentation: 0x3 (XEN) [ 22.786161] - PPR Log Overflow Early Warning (XEN) [ 22.825552] - PPR Automatic Response (XEN) [ 22.864211] - Memory Access Routing and Control: 0x1 (XEN) [ 22.904917] - Block StopMark Message (XEN) [ 22.944075] - Performance Optimization (XEN) [ 22.983360] - MSI Capability MMIO Access (XEN) [ 23.022815] - Guest I/O Protection (XEN) [ 23.061548] - Host Access (XEN) [ 23.099437] - Enhanced PPR Handling (XEN) [ 23.138461] - Attribute Forward (XEN) [ 23.177008] - Virtualized IOMMU (XEN) [ 23.215523] - VMGuard I/O Support (XEN) [ 23.254043] - VM Table Size: 0x2 (XEN) [ 23.295705] AMD-Vi: IOMMU Extended Features: (XEN) [ 23.335250] - Peripheral Page Service Request (XEN) [ 23.374941] - x2APIC (XEN) [ 23.411860] - NX bit (XEN) [ 23.448460] - Invalidate All Command (XEN) [ 23.486748] - Guest APIC (XEN) [ 23.523569] - Performance Counters (XEN) [ 23.561564] - Host Address Translation Size: 0x2 (XEN) [ 23.601127] - Guest Address Translation Size: 0 (XEN) [ 23.640584] - Guest CR3 Root Table Level: 0x1 (XEN) [ 23.679885] - Maximum PASID: 0xf (XEN) [ 23.717689] - SMI Filter Register: 0x1 (XEN) [ 23.755994] - SMI Filter Register Count: 0x2 (XEN) [ 23.794897] - Guest Virtual APIC Modes: 0x1 (XEN) [ 23.833766] - Dual PPR Log: 0x2 (XEN) [ 23.871319] - Dual Event Log: 0x2 (XEN) [ 23.909008] - User / Supervisor Page Protection (XEN) [ 23.948434] - Device Table Segmentation: 0x3 (XEN) [ 23.987698] - PPR Log Overflow Early Warning (XEN) [ 24.027099] - PPR Automatic Response (XEN) [ 24.065748] - Memory Access Routing and Control: 0x1 (XEN) [ 24.106438] - Block StopMark Message (XEN) [ 24.145591] - Performance Optimization (XEN) [ 24.184893] - MSI Capability MMIO Access (XEN) [ 24.224345] - Guest I/O Protection (XEN) [ 24.263076] - Host Access (XEN) [ 24.300974] - Enhanced PPR Handling (XEN) [ 24.339982] - Attribute Forward (XEN) [ 24.378528] - Virtualized IOMMU (XEN) [ 24.417041] - VMGuard I/O Support (XEN) [ 24.455584] - VM Table Size: 0x2 (XEN) [ 24.497260] AMD-Vi: IOMMU Extended Features: (XEN) [ 24.536794] - Peripheral Page Service Request (XEN) [ 24.576469] - x2APIC (XEN) [ 24.613395] - NX bit (XEN) [ 24.649996] - Invalidate All Command (XEN) [ 24.688294] - Guest APIC (XEN) [ 24.725142] - Performance Counters (XEN) [ 24.763127] - Host Address Translation Size: 0x2 (XEN) [ 24.802707] - Guest Address Translation Size: 0 (XEN) [ 24.842172] - Guest CR3 Root Table Level: 0x1 (XEN) [ 24.881459] - Maximum PASID: 0xf (XEN) [ 24.919288] - SMI Filter Register: 0x1 (XEN) [ 24.957583] - SMI Filter Register Count: 0x2 (XEN) [ 24.996496] - Guest Virtual APIC Modes: 0x1 (XEN) [ 25.035364] - Dual PPR Log: 0x2 (XEN) [ 25.072908] - Dual Event Log: 0x2 (XEN) [ 25.110588] - User / Supervisor Page Protection (XEN) [ 25.150037] - Device Table Segmentation: 0x3 (XEN) [ 25.189290] - PPR Log Overflow Early Warning (XEN) [ 25.228696] - PPR Automatic Response (XEN) [ 25.267348] - Memory Access Routing a in Xennd Control: 0x1 (XEN) [ 25.308045] - Block StopMark Message (XEN) [ 25.347199] - Performance Optimization (XEN) [ 25.386484] - MSI Capability MMIO Access (XEN) [ 25.425950] - Guest I/O Protection (XEN) [ 25.464680] - Host Access (XEN) [ 25.502580] - Enhanced PPR Handling (XEN) [ 25.541588] - Attribute Forward (XEN) [ 25.580145] - Virtualized IOMMU (XEN) [ 25.618647] - VMGuard I/O Support (XEN) [ 25.657196] - VM Table Size: 0x2 (XEN) [ 25.698883] AMD-Vi: IOMMU Extended Features: (XEN) [ 25.738420] - Peripheral Page Service Request (XEN) [ 25.778084] - x2APIC (XEN) [ 25.815010] - NX bit (XEN) [ 25.851603] - Invalidate All Command (XEN) [ 25.889907] - Guest APIC (XEN) [ 25.926756] - Performance Counters (XEN) [ 25.964742] - Host Address Translation Size: 0x2 (XEN) [ 26.004312] - Guest Address Translation Size: 0 (XEN) [ 26.043787] - Guest CR3 Root Table Level: 0x1 (XEN) [ 26.083064] - Maximum PASID: 0xf (XEN) [ 26.120903] - SMI Filter Register: 0x1 (XEN) [ 26.159215] - SMI Filter Register Count: 0x2 (XEN) [ 26.198129] - Guest Virtual APIC Modes: 0x1 (XEN) [ 26.237015] - Dual PPR Log: 0x2 (XEN) [ 26.274565] - Dual Event Log: 0x2 (XEN) [ 26.312231] - User / Supervisor Page Protection (XEN) [ 26.351672] - Device Table Segmentation: 0x3 (XEN) [ 26.390949] - PPR Log Overflow Early Warning (XEN) [ 26.430343] - PPR Automatic Response (XEN) [ 26.469006] - Memory Access Routing and Control: 0x1 (XEN) [ 26.509711] - Block StopMark Message (XEN) [ 26.548867] - Performance Optimization (XEN) [ 26.588159] - MSI Capability MMIO Access (XEN) [ 26.627627] - Guest I/O Protection (XEN) [ 26.666355] - Host Access (XEN) [ 26.704253] - Enhanced PPR Handling (XEN) [ 26.743271] - Attribute Forward (XEN) [ 26.781828] - Virtualized IOMMU (XEN) [ 26.820341] - VMGuard I/O Support (XEN) [ 26.858874] - VM Table Size: 0x2 (XEN) [ 26.900549] AMD-Vi: IOMMU Extended Features: (XEN) [ 26.940095] - Peripheral Page Service Request (XEN) [ 26.979769] - x2APIC (XEN) [ 27.016686] - NX bit (XEN) [ 27.053287] - Invalidate All Command (XEN) [ 27.091582] - Guest APIC (XEN) [ 27.128432] - Performance Counters (XEN) [ 27.166418] - Host Address Translation Size: 0x2 (XEN) [ 27.205971] - Guest Address Translation Size: 0 (XEN) [ 27.245440] - Guest CR3 Root Table Level: 0x1 (XEN) [ 27.284714] - Maximum PASID: 0xf (XEN) [ 27.322543] - SMI Filter Register: 0x1 (XEN) [ 27.360847] - SMI Filter Register Count: 0x2 (XEN) [ 27.399768] - Guest Virtual APIC Modes: 0x1 (XEN) [ 27.438656] - Dual PPR Log: 0x2 (XEN) [ 27.476209] - Dual Event Log: 0x2 (XEN) [ 27.513889] - User / Supervisor Page Protection (XEN) [ 27.553331] - Device Table Segmentation: 0x3 (XEN) [ 27.592595] - PPR Log Overflow Early Warning (XEN) [ 27.632003] - PPR Automatic Response (XEN) [ 27.670655] - Memory Access Routing and Control: 0x1 (XEN) [ 27.711351] - Block StopMark Message (XEN) [ 27.750525] - Performance Optimization (XEN) [ 27.789811] - MSI Capability MMIO Access (XEN) [ 27.829267] - Guest I/O Protection (XEN) [ 27.868005] - Host Access (XEN) [ 27.905889] - Enhanced PPR Handling (XEN) [ 27.944896] - Attribute Forward (XEN) [ 27.983445] - Virtualized IOMMU (XEN) [ 28.021956] - VMGuard I/O Support (XEN) [ 28.060494] - VM Table Size: 0x2 (XEN) [ 28.284879] AMD-Vi: Disabled HAP memory map sharing with IOMMU (XEN) [ 28.326856] AMD-Vi: IOMMU 0 Enabled. (XEN) [ 28.366011] AMD-Vi: IOMMU 1 Enabled. (XEN) [ 28.405142] AMD-Vi: IOMMU 2 Enabled. (XEN) [ 28.444150] AMD-Vi: IOMMU 3 Enabled. (XEN) [ 28.483062] AMD-Vi: IOMMU 4 Enabled. (XEN) [ 28.521923] AMD-Vi: IOMMU 5 Enabled. (XEN) [ 28.560798] AMD-Vi: IOMMU 6 Enabled. (XEN) [ 28.599528] AMD-Vi: IOMMU 7 Enabled. (XEN) [ 28.645382] I/O virtualisation enabled (XEN) [ 28.684034] - Dom0 mode: Relaxed (XEN) [ 28.722063] Interrupt remapping enabled Given that the expected case is that all IOMMUs are identical, how about only printing the details for IOMMU0, and eliding printing for further IOMMUs which have an identical featureset? > --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h > @@ -346,26 +346,57 @@ struct amd_iommu_dte { > -#define IOMMU_EXT_FEATURE_PASMAX_SHIFT 0x0 > -#define IOMMU_EXT_FEATURE_PASMAX_MASK 0x0000001F > +union amd_iommu_ext_features { > + uint64_t raw; > + struct { > + unsigned int pref_sup:1; > + unsigned int ppr_sup:1; > + unsigned int xt_sup:1; > + unsigned int nx_sup:1; > + unsigned int gt_sup:1; > + unsigned int gappi_sup:1; > + unsigned int ia_sup:1; > + unsigned int ga_sup:1; > + unsigned int he_sup:1; > + unsigned int pc_sup:1; > + unsigned int hats:2; > + unsigned int gats:2; > + unsigned int glx_sup:2; > + unsigned int smif_sup:2; > + unsigned int smif_rc:3; > + unsigned int gam_sup:3; > + unsigned int dual_ppr_log_sup:2; > + unsigned int :2; > + unsigned int dual_event_log_sup:2; > + unsigned int sat_sup:1; > + unsigned int :1; > + unsigned int pas_max:5; > + unsigned int us_sup:1; > + unsigned int dev_tbl_seg_sup:2; > + unsigned int ppr_early_of_sup:1; > + unsigned int ppr_auto_rsp_sup:1; > + unsigned int marc_sup:2; > + unsigned int blk_stop_mrk_sup:1; > + unsigned int perf_opt_sup:1; > + unsigned int msi_cap_mmio_sup:1; > + unsigned int :1; > + unsigned int gio_sup:1; > + unsigned int ha_sup:1; > + unsigned int eph_sup:1; > + unsigned int attr_fw_sup:1; > + unsigned int hd_sup:1; > + unsigned int :1; > + unsigned int inv_iotlb_type_sup:1; > + unsigned int viommu_sup:1; > + unsigned int vm_guard_io_sup:1; > + unsigned int vm_table_size:4; > + unsigned int ga_update_dis_sup:1; > + unsigned int :2; > + } flds; > +}; I'd suggest bool for single bitfields. We've been bitten multiple times by "x = (y & 0x80)" type bugs, which truncate to 0 using unsigned int bitfields, but correctly become 1 given bool bitfields. ~Andrew
>>> On 17.06.19 at 22:23, <andrew.cooper3@citrix.com> wrote: > On 13/06/2019 14:22, Jan Beulich wrote: >> This also takes care of several of the shift values wrongly having been >> specified as hex rather than dec. >> >> Take the opportunity and add further fields. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> >> --- a/xen/drivers/passthrough/amd/iommu_detect.c >> +++ b/xen/drivers/passthrough/amd/iommu_detect.c >> @@ -60,43 +60,72 @@ static int __init get_iommu_capabilities >> >> void __init get_iommu_features(struct amd_iommu *iommu) >> { >> - u32 low, high; >> - int i = 0 ; >> - static const char *__initdata feature_str[] = { >> - "- Prefetch Pages Command", >> - "- Peripheral Page Service Request", >> - "- X2APIC Supported", >> - "- NX bit Supported", >> - "- Guest Translation", >> - "- Reserved bit [5]", >> - "- Invalidate All Command", >> - "- Guest APIC supported", >> - "- Hardware Error Registers", >> - "- Performance Counters", >> - NULL >> - }; >> - >> ASSERT( iommu->mmio_base ); >> >> if ( !iommu_has_cap(iommu, PCI_CAP_EFRSUP_SHIFT) ) >> { >> - iommu->features = 0; >> + iommu->features.raw = 0; >> return; >> } >> >> - low = readl(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET); >> - high = readl(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET + 4); >> - >> - iommu->features = ((u64)high << 32) | low; >> + iommu->features.raw = >> + readq(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET); >> >> printk("AMD-Vi: IOMMU Extended Features:\n"); >> >> - while ( feature_str[i] ) >> +#define MASK(fld) ((union amd_iommu_ext_features){ .flds.fld = ~0 }).raw >> +#define FEAT(fld, str) do { \ >> + if ( MASK(fld) & (MASK(fld) - 1) ) \ >> + printk( "- " str ": %#x\n", iommu->features.flds.fld); \ >> + else if ( iommu->features.raw & MASK(fld) ) \ >> + printk( "- " str "\n"); \ >> +} while ( false ) >> + >> + FEAT(pref_sup, "Prefetch Pages Command"); >> + FEAT(ppr_sup, "Peripheral Page Service Request"); >> + FEAT(xt_sup, "x2APIC"); >> + FEAT(nx_sup, "NX bit"); >> + FEAT(gappi_sup, "Guest APIC Physical Processor Interrupt"); >> + FEAT(ia_sup, "Invalidate All Command"); >> + FEAT(ga_sup, "Guest APIC"); >> + FEAT(he_sup, "Hardware Error Registers"); >> + FEAT(pc_sup, "Performance Counters"); >> + FEAT(hats, "Host Address Translation Size"); >> + >> + if ( iommu->features.flds.gt_sup ) >> { >> - if ( amd_iommu_has_feature(iommu, i) ) >> - printk( " %s\n", feature_str[i]); >> - i++; >> + FEAT(gats, "Guest Address Translation Size"); >> + FEAT(glx_sup, "Guest CR3 Root Table Level"); >> + FEAT(pas_max, "Maximum PASID"); >> } >> + >> + FEAT(smif_sup, "SMI Filter Register"); >> + FEAT(smif_rc, "SMI Filter Register Count"); >> + FEAT(gam_sup, "Guest Virtual APIC Modes"); >> + FEAT(dual_ppr_log_sup, "Dual PPR Log"); >> + FEAT(dual_event_log_sup, "Dual Event Log"); >> + FEAT(sat_sup, "Secure ATS"); >> + FEAT(us_sup, "User / Supervisor Page Protection"); >> + FEAT(dev_tbl_seg_sup, "Device Table Segmentation"); >> + FEAT(ppr_early_of_sup, "PPR Log Overflow Early Warning"); >> + FEAT(ppr_auto_rsp_sup, "PPR Automatic Response"); >> + FEAT(marc_sup, "Memory Access Routing and Control"); >> + FEAT(blk_stop_mrk_sup, "Block StopMark Message"); >> + FEAT(perf_opt_sup , "Performance Optimization"); >> + FEAT(msi_cap_mmio_sup, "MSI Capability MMIO Access"); >> + FEAT(gio_sup, "Guest I/O Protection"); >> + FEAT(ha_sup, "Host Access"); >> + FEAT(eph_sup, "Enhanced PPR Handling"); >> + FEAT(attr_fw_sup, "Attribute Forward"); >> + FEAT(hd_sup, "Host Dirty"); >> + FEAT(inv_iotlb_type_sup, "Invalidate IOTLB Type"); >> + FEAT(viommu_sup, "Virtualized IOMMU"); >> + FEAT(vm_guard_io_sup, "VMGuard I/O Support"); >> + FEAT(vm_table_size, "VM Table Size"); >> + FEAT(ga_update_dis_sup, "Guest Access Bit Update Disable"); >> + >> +#undef FEAT >> +#undef MASK >> } > > So this is fine, but does come with a downside. This is the log from a > Rome system: > > (XEN) [ 17.928225] AMD-Vi: IOMMU Extended Features: > (XEN) [ 17.988264] - Peripheral Page Service Request > (XEN) [ 18.048082] - x2APIC > (XEN) [ 18.104735] - NX bit > (XEN) [ 18.160751] - Invalidate All Command > (XEN) [ 18.218116] - Guest APIC > (XEN) [ 18.273660] - Performance Counters > (XEN) [ 18.329868] - Host Address Translation Size: 0x2 > (XEN) [ 18.387363] - Guest Address Translation Size: 0 > (XEN) [ 18.444446] - Guest CR3 Root Table Level: 0x1 > (XEN) [ 18.501006] - Maximum PASID: 0xf > (XEN) [ 18.555753] - SMI Filter Register: 0x1 > (XEN) [ 18.610773] - SMI Filter Register Count: 0x2 > (XEN) [ 18.666116] - Guest Virtual APIC Modes: 0x1 > (XEN) [ 18.721036] - Dual PPR Log: 0x2 > (XEN) [ 18.774237] - Dual Event Log: 0x2 > (XEN) [ 18.827164] - User / Supervisor Page Protection > (XEN) [ 18.881374] - Device Table Segmentation: 0x3 > (XEN) [ 18.934949] - PPR Log Overflow Early Warning > (XEN) [ 18.988186] - PPR Automatic Response > (XEN) [ 19.040193] - Memory Access Routing and Control: 0x1 > (XEN) [ 19.093770] - Block StopMark Message > (XEN) [ 19.145300] - Performance Optimization > (XEN) [ 19.196603] - MSI Capability MMIO Access > (XEN) [ 19.247754] - Guest I/O Protection > (XEN) [ 19.297811] - Host Access > (XEN) [ 19.346396] - Enhanced PPR Handling > (XEN) [ 19.395647] - Attribute Forward > (XEN) [ 19.443986] - Virtualized IOMMU > (XEN) [ 19.491828] - VMGuard I/O Support > (XEN) [ 19.539404] - VM Table Size: 0x2 > (XEN) [ 19.589837] AMD-Vi: IOMMU Extended Features: > (XEN) [ 19.637829] - Peripheral Page Service Request > (XEN) [ 19.685607] - x2APIC > (XEN) [ 19.730231] - NX bit > (XEN) [ 19.774195] - Invalidate All Command > (XEN) [ 19.819528] - Guest APIC > (XEN) [ 19.863027] - Performance Counters > (XEN) [ 19.907165] - Host Address Translation Size: 0x2 > (XEN) [ 19.952578] - Guest Address Translation Size: 0 > (XEN) [ 19.997588] - Guest CR3 Root Table Level: 0x1 > (XEN) [ 20.042057] - Maximum PASID: 0xf > (XEN) [ 20.084732] - SMI Filter Register: 0x1 > (XEN) [ 20.127648] - SMI Filter Register Count: 0x2 > (XEN) [ 20.170910] - Guest Virtual APIC Modes: 0x1 > (XEN) [ 20.213702] - Dual PPR Log: 0x2 > (XEN) [ 20.254765] - Dual Event Log: 0x2 > (XEN) [ 20.295567] - User / Supervisor Page Protection > (XEN) [ 20.337626] - Device Table Segmentation: 0x3 > (XEN) [ 20.379042] - PPR Log Overflow Early Warning > (XEN) [ 20.420094] - PPR Automatic Response > (XEN) [ 20.459890] - Memory Access Routing and Control: 0x1 > (XEN) [ 20.501248] - Block StopMark Message > (XEN) [ 20.540577] - Performance Optimization > (XEN) [ 20.579876] - MSI Capability MMIO Access > (XEN) [ 20.619309] - Guest I/O Protection > (XEN) [ 20.658041] - Host Access > (XEN) [ 20.695937] - Enhanced PPR Handling > (XEN) [ 20.735276] - Attribute Forward > (XEN) [ 20.773830] - Virtualized IOMMU > (XEN) [ 20.812335] - VMGuard I/O Support > (XEN) [ 20.850868] - VM Table Size: 0x2 > (XEN) [ 20.892543] AMD-Vi: IOMMU Extended Features: > (XEN) [ 20.932087] - Peripheral Page Service Request > (XEN) [ 20.971756] - x2APIC > (XEN) [ 21.008681] - NX bit > (XEN) [ 21.045279] - Invalidate All Command > (XEN) [ 21.083567] - Guest APIC > (XEN) [ 21.120410] - Performance Counters > (XEN) [ 21.158386] - Host Address Translation Size: 0x2 > (XEN) [ 21.197956] - Guest Address Translation Size: 0 > (XEN) [ 21.237433] - Guest CR3 Root Table Level: 0x1 > (XEN) [ 21.276709] - Maximum PASID: 0xf > (XEN) [ 21.314538] - SMI Filter Register: 0x1 > (XEN) [ 21.352844] - SMI Filter Register Count: 0x2 > (XEN) [ 21.391728] - Guest Virtual APIC Modes: 0x1 > (XEN) [ 21.430614] - Dual PPR Log: 0x2 > (XEN) [ 21.468150] - Dual Event Log: 0x2 > (XEN) [ 21.505833] - User / Supervisor Page Protection > (XEN) [ 21.545270] - Device Table Segmentation: 0x3 > (XEN) [ 21.584538] - PPR Log Overflow Early Warning > (XEN) [ 21.623928] - PPR Automatic Response > (XEN) [ 21.662564] - Memory Access Routing and Control: 0x1 > (XEN) [ 21.703268] - Block StopMark Message > (XEN) [ 21.742415] - Performance Optimization > (XEN) [ 21.781692] - MSI Capability MMIO Access > (XEN) [ 21.821159] - Guest I/O Protection > (XEN) [ 21.859916] - Host Access > (XEN) [ 21.897811] - Enhanced PPR Handling > (XEN) [ 21.936818] - Attribute Forward > (XEN) [ 21.975387] - Virtualized IOMMU > (XEN) [ 22.013898] - VMGuard I/O Support > (XEN) [ 22.052448] - VM Table Size: 0x2 > (XEN) [ 22.094140] AMD-Vi: IOMMU Extended Features: > (XEN) [ 22.133685] - Peripheral Page Service Request > (XEN) [ 22.173363] - x2APIC > (XEN) [ 22.210287] - NX bit > (XEN) [ 22.246888] - Invalidate All Command > (XEN) [ 22.285176] - Guest APIC > (XEN) [ 22.322008] - Performance Counters > (XEN) [ 22.359994] - Host Address Translation Size: 0x2 > (XEN) [ 22.399552] - Guest Address Translation Size: 0 > (XEN) [ 22.439028] - Guest CR3 Root Table Level: 0x1 > (XEN) [ 22.478307] - Maximum PASID: 0xf > (XEN) [ 22.516133] - SMI Filter Register: 0x1 > (XEN) [ 22.554441] - SMI Filter Register Count: 0x2 > (XEN) [ 22.593345] - Guest Virtual APIC Modes: 0x1 > (XEN) [ 22.632221] - Dual PPR Log: 0x2 > (XEN) [ 22.669766] - Dual Event Log: 0x2 > (XEN) [ 22.707455] - User / Supervisor Page Protection > (XEN) [ 22.746896] - Device Table Segmentation: 0x3 > (XEN) [ 22.786161] - PPR Log Overflow Early Warning > (XEN) [ 22.825552] - PPR Automatic Response > (XEN) [ 22.864211] - Memory Access Routing and Control: 0x1 > (XEN) [ 22.904917] - Block StopMark Message > (XEN) [ 22.944075] - Performance Optimization > (XEN) [ 22.983360] - MSI Capability MMIO Access > (XEN) [ 23.022815] - Guest I/O Protection > (XEN) [ 23.061548] - Host Access > (XEN) [ 23.099437] - Enhanced PPR Handling > (XEN) [ 23.138461] - Attribute Forward > (XEN) [ 23.177008] - Virtualized IOMMU > (XEN) [ 23.215523] - VMGuard I/O Support > (XEN) [ 23.254043] - VM Table Size: 0x2 > (XEN) [ 23.295705] AMD-Vi: IOMMU Extended Features: > (XEN) [ 23.335250] - Peripheral Page Service Request > (XEN) [ 23.374941] - x2APIC > (XEN) [ 23.411860] - NX bit > (XEN) [ 23.448460] - Invalidate All Command > (XEN) [ 23.486748] - Guest APIC > (XEN) [ 23.523569] - Performance Counters > (XEN) [ 23.561564] - Host Address Translation Size: 0x2 > (XEN) [ 23.601127] - Guest Address Translation Size: 0 > (XEN) [ 23.640584] - Guest CR3 Root Table Level: 0x1 > (XEN) [ 23.679885] - Maximum PASID: 0xf > (XEN) [ 23.717689] - SMI Filter Register: 0x1 > (XEN) [ 23.755994] - SMI Filter Register Count: 0x2 > (XEN) [ 23.794897] - Guest Virtual APIC Modes: 0x1 > (XEN) [ 23.833766] - Dual PPR Log: 0x2 > (XEN) [ 23.871319] - Dual Event Log: 0x2 > (XEN) [ 23.909008] - User / Supervisor Page Protection > (XEN) [ 23.948434] - Device Table Segmentation: 0x3 > (XEN) [ 23.987698] - PPR Log Overflow Early Warning > (XEN) [ 24.027099] - PPR Automatic Response > (XEN) [ 24.065748] - Memory Access Routing and Control: 0x1 > (XEN) [ 24.106438] - Block StopMark Message > (XEN) [ 24.145591] - Performance Optimization > (XEN) [ 24.184893] - MSI Capability MMIO Access > (XEN) [ 24.224345] - Guest I/O Protection > (XEN) [ 24.263076] - Host Access > (XEN) [ 24.300974] - Enhanced PPR Handling > (XEN) [ 24.339982] - Attribute Forward > (XEN) [ 24.378528] - Virtualized IOMMU > (XEN) [ 24.417041] - VMGuard I/O Support > (XEN) [ 24.455584] - VM Table Size: 0x2 > (XEN) [ 24.497260] AMD-Vi: IOMMU Extended Features: > (XEN) [ 24.536794] - Peripheral Page Service Request > (XEN) [ 24.576469] - x2APIC > (XEN) [ 24.613395] - NX bit > (XEN) [ 24.649996] - Invalidate All Command > (XEN) [ 24.688294] - Guest APIC > (XEN) [ 24.725142] - Performance Counters > (XEN) [ 24.763127] - Host Address Translation Size: 0x2 > (XEN) [ 24.802707] - Guest Address Translation Size: 0 > (XEN) [ 24.842172] - Guest CR3 Root Table Level: 0x1 > (XEN) [ 24.881459] - Maximum PASID: 0xf > (XEN) [ 24.919288] - SMI Filter Register: 0x1 > (XEN) [ 24.957583] - SMI Filter Register Count: 0x2 > (XEN) [ 24.996496] - Guest Virtual APIC Modes: 0x1 > (XEN) [ 25.035364] - Dual PPR Log: 0x2 > (XEN) [ 25.072908] - Dual Event Log: 0x2 > (XEN) [ 25.110588] - User / Supervisor Page Protection > (XEN) [ 25.150037] - Device Table Segmentation: 0x3 > (XEN) [ 25.189290] - PPR Log Overflow Early Warning > (XEN) [ 25.228696] - PPR Automatic Response > (XEN) [ 25.267348] - Memory Access Routing a in Xennd Control: 0x1 > (XEN) [ 25.308045] - Block StopMark Message > (XEN) [ 25.347199] - Performance Optimization > (XEN) [ 25.386484] - MSI Capability MMIO Access > (XEN) [ 25.425950] - Guest I/O Protection > (XEN) [ 25.464680] - Host Access > (XEN) [ 25.502580] - Enhanced PPR Handling > (XEN) [ 25.541588] - Attribute Forward > (XEN) [ 25.580145] - Virtualized IOMMU > (XEN) [ 25.618647] - VMGuard I/O Support > (XEN) [ 25.657196] - VM Table Size: 0x2 > (XEN) [ 25.698883] AMD-Vi: IOMMU Extended Features: > (XEN) [ 25.738420] - Peripheral Page Service Request > (XEN) [ 25.778084] - x2APIC > (XEN) [ 25.815010] - NX bit > (XEN) [ 25.851603] - Invalidate All Command > (XEN) [ 25.889907] - Guest APIC > (XEN) [ 25.926756] - Performance Counters > (XEN) [ 25.964742] - Host Address Translation Size: 0x2 > (XEN) [ 26.004312] - Guest Address Translation Size: 0 > (XEN) [ 26.043787] - Guest CR3 Root Table Level: 0x1 > (XEN) [ 26.083064] - Maximum PASID: 0xf > (XEN) [ 26.120903] - SMI Filter Register: 0x1 > (XEN) [ 26.159215] - SMI Filter Register Count: 0x2 > (XEN) [ 26.198129] - Guest Virtual APIC Modes: 0x1 > (XEN) [ 26.237015] - Dual PPR Log: 0x2 > (XEN) [ 26.274565] - Dual Event Log: 0x2 > (XEN) [ 26.312231] - User / Supervisor Page Protection > (XEN) [ 26.351672] - Device Table Segmentation: 0x3 > (XEN) [ 26.390949] - PPR Log Overflow Early Warning > (XEN) [ 26.430343] - PPR Automatic Response > (XEN) [ 26.469006] - Memory Access Routing and Control: 0x1 > (XEN) [ 26.509711] - Block StopMark Message > (XEN) [ 26.548867] - Performance Optimization > (XEN) [ 26.588159] - MSI Capability MMIO Access > (XEN) [ 26.627627] - Guest I/O Protection > (XEN) [ 26.666355] - Host Access > (XEN) [ 26.704253] - Enhanced PPR Handling > (XEN) [ 26.743271] - Attribute Forward > (XEN) [ 26.781828] - Virtualized IOMMU > (XEN) [ 26.820341] - VMGuard I/O Support > (XEN) [ 26.858874] - VM Table Size: 0x2 > (XEN) [ 26.900549] AMD-Vi: IOMMU Extended Features: > (XEN) [ 26.940095] - Peripheral Page Service Request > (XEN) [ 26.979769] - x2APIC > (XEN) [ 27.016686] - NX bit > (XEN) [ 27.053287] - Invalidate All Command > (XEN) [ 27.091582] - Guest APIC > (XEN) [ 27.128432] - Performance Counters > (XEN) [ 27.166418] - Host Address Translation Size: 0x2 > (XEN) [ 27.205971] - Guest Address Translation Size: 0 > (XEN) [ 27.245440] - Guest CR3 Root Table Level: 0x1 > (XEN) [ 27.284714] - Maximum PASID: 0xf > (XEN) [ 27.322543] - SMI Filter Register: 0x1 > (XEN) [ 27.360847] - SMI Filter Register Count: 0x2 > (XEN) [ 27.399768] - Guest Virtual APIC Modes: 0x1 > (XEN) [ 27.438656] - Dual PPR Log: 0x2 > (XEN) [ 27.476209] - Dual Event Log: 0x2 > (XEN) [ 27.513889] - User / Supervisor Page Protection > (XEN) [ 27.553331] - Device Table Segmentation: 0x3 > (XEN) [ 27.592595] - PPR Log Overflow Early Warning > (XEN) [ 27.632003] - PPR Automatic Response > (XEN) [ 27.670655] - Memory Access Routing and Control: 0x1 > (XEN) [ 27.711351] - Block StopMark Message > (XEN) [ 27.750525] - Performance Optimization > (XEN) [ 27.789811] - MSI Capability MMIO Access > (XEN) [ 27.829267] - Guest I/O Protection > (XEN) [ 27.868005] - Host Access > (XEN) [ 27.905889] - Enhanced PPR Handling > (XEN) [ 27.944896] - Attribute Forward > (XEN) [ 27.983445] - Virtualized IOMMU > (XEN) [ 28.021956] - VMGuard I/O Support > (XEN) [ 28.060494] - VM Table Size: 0x2 > (XEN) [ 28.284879] AMD-Vi: Disabled HAP memory map sharing with IOMMU > (XEN) [ 28.326856] AMD-Vi: IOMMU 0 Enabled. > (XEN) [ 28.366011] AMD-Vi: IOMMU 1 Enabled. > (XEN) [ 28.405142] AMD-Vi: IOMMU 2 Enabled. > (XEN) [ 28.444150] AMD-Vi: IOMMU 3 Enabled. > (XEN) [ 28.483062] AMD-Vi: IOMMU 4 Enabled. > (XEN) [ 28.521923] AMD-Vi: IOMMU 5 Enabled. > (XEN) [ 28.560798] AMD-Vi: IOMMU 6 Enabled. > (XEN) [ 28.599528] AMD-Vi: IOMMU 7 Enabled. > (XEN) [ 28.645382] I/O virtualisation enabled > (XEN) [ 28.684034] - Dom0 mode: Relaxed > (XEN) [ 28.722063] Interrupt remapping enabled > > Given that the expected case is that all IOMMUs are identical, how about > only printing the details for IOMMU0, and eliding printing for further > IOMMUs which have an identical featureset? Obviously I did notice this too. I can add a patch to the series to improve the situation (perhaps even ahead of this one), but I don't think I want to do this right here. >> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h >> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h >> @@ -346,26 +346,57 @@ struct amd_iommu_dte { >> -#define IOMMU_EXT_FEATURE_PASMAX_SHIFT 0x0 >> -#define IOMMU_EXT_FEATURE_PASMAX_MASK 0x0000001F >> +union amd_iommu_ext_features { >> + uint64_t raw; >> + struct { >> + unsigned int pref_sup:1; >> + unsigned int ppr_sup:1; >> + unsigned int xt_sup:1; >> + unsigned int nx_sup:1; >> + unsigned int gt_sup:1; >> + unsigned int gappi_sup:1; >> + unsigned int ia_sup:1; >> + unsigned int ga_sup:1; >> + unsigned int he_sup:1; >> + unsigned int pc_sup:1; >> + unsigned int hats:2; >> + unsigned int gats:2; >> + unsigned int glx_sup:2; >> + unsigned int smif_sup:2; >> + unsigned int smif_rc:3; >> + unsigned int gam_sup:3; >> + unsigned int dual_ppr_log_sup:2; >> + unsigned int :2; >> + unsigned int dual_event_log_sup:2; >> + unsigned int sat_sup:1; >> + unsigned int :1; >> + unsigned int pas_max:5; >> + unsigned int us_sup:1; >> + unsigned int dev_tbl_seg_sup:2; >> + unsigned int ppr_early_of_sup:1; >> + unsigned int ppr_auto_rsp_sup:1; >> + unsigned int marc_sup:2; >> + unsigned int blk_stop_mrk_sup:1; >> + unsigned int perf_opt_sup:1; >> + unsigned int msi_cap_mmio_sup:1; >> + unsigned int :1; >> + unsigned int gio_sup:1; >> + unsigned int ha_sup:1; >> + unsigned int eph_sup:1; >> + unsigned int attr_fw_sup:1; >> + unsigned int hd_sup:1; >> + unsigned int :1; >> + unsigned int inv_iotlb_type_sup:1; >> + unsigned int viommu_sup:1; >> + unsigned int vm_guard_io_sup:1; >> + unsigned int vm_table_size:4; >> + unsigned int ga_update_dis_sup:1; >> + unsigned int :2; >> + } flds; >> +}; > > I'd suggest bool for single bitfields. We've been bitten multiple times > by "x = (y & 0x80)" type bugs, which truncate to 0 using unsigned int > bitfields, but correctly become 1 given bool bitfields. Oh, it took me a while to figure what you mean - you're after the case of x (in your example), not y being a bitfield reference. I don't think we're at risk of introducing such constructs here, so personally I'd prefer it to stay as it is, but I'd listen to Brian and/or Suravee leaning more towards your suggestion. Jan
>>> On 17.06.19 at 21:07, <Brian.Woods@amd.com> wrote: > On Thu, Jun 13, 2019 at 07:22:31AM -0600, Jan Beulich wrote: >> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h >> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h >> @@ -346,26 +346,57 @@ struct amd_iommu_dte { >> #define IOMMU_EXCLUSION_LIMIT_HIGH_MASK 0xFFFFFFFF >> #define IOMMU_EXCLUSION_LIMIT_HIGH_SHIFT 0 >> >> -/* Extended Feature Register*/ >> +/* Extended Feature Register */ >> #define IOMMU_EXT_FEATURE_MMIO_OFFSET 0x30 >> -#define IOMMU_EXT_FEATURE_PREFSUP_SHIFT 0x0 >> -#define IOMMU_EXT_FEATURE_PPRSUP_SHIFT 0x1 >> -#define IOMMU_EXT_FEATURE_XTSUP_SHIFT 0x2 >> -#define IOMMU_EXT_FEATURE_NXSUP_SHIFT 0x3 >> -#define IOMMU_EXT_FEATURE_GTSUP_SHIFT 0x4 >> -#define IOMMU_EXT_FEATURE_IASUP_SHIFT 0x6 >> -#define IOMMU_EXT_FEATURE_GASUP_SHIFT 0x7 >> -#define IOMMU_EXT_FEATURE_HESUP_SHIFT 0x8 >> -#define IOMMU_EXT_FEATURE_PCSUP_SHIFT 0x9 >> -#define IOMMU_EXT_FEATURE_HATS_SHIFT 0x10 >> -#define IOMMU_EXT_FEATURE_HATS_MASK 0x00000C00 >> -#define IOMMU_EXT_FEATURE_GATS_SHIFT 0x12 >> -#define IOMMU_EXT_FEATURE_GATS_MASK 0x00003000 >> -#define IOMMU_EXT_FEATURE_GLXSUP_SHIFT 0x14 >> -#define IOMMU_EXT_FEATURE_GLXSUP_MASK 0x0000C000 >> >> -#define IOMMU_EXT_FEATURE_PASMAX_SHIFT 0x0 >> -#define IOMMU_EXT_FEATURE_PASMAX_MASK 0x0000001F >> +union amd_iommu_ext_features { >> + uint64_t raw; >> + struct { >> + unsigned int pref_sup:1; >> + unsigned int ppr_sup:1; >> + unsigned int xt_sup:1; >> + unsigned int nx_sup:1; >> + unsigned int gt_sup:1; >> + unsigned int gappi_sup:1; >> + unsigned int ia_sup:1; >> + unsigned int ga_sup:1; >> + unsigned int he_sup:1; >> + unsigned int pc_sup:1; >> + unsigned int hats:2; >> + unsigned int gats:2; >> + unsigned int glx_sup:2; >> + unsigned int smif_sup:2; >> + unsigned int smif_rc:3; >> + unsigned int gam_sup:3; >> + unsigned int dual_ppr_log_sup:2; >> + unsigned int :2; >> + unsigned int dual_event_log_sup:2; > >> + unsigned int sat_sup:1; >> + unsigned int :1; > I think these might be flipped. Oh, indeed. And I've also omitted an 's' from the name. Thanks for noticing. Jan
--- a/xen/drivers/passthrough/amd/iommu_detect.c +++ b/xen/drivers/passthrough/amd/iommu_detect.c @@ -60,43 +60,72 @@ static int __init get_iommu_capabilities void __init get_iommu_features(struct amd_iommu *iommu) { - u32 low, high; - int i = 0 ; - static const char *__initdata feature_str[] = { - "- Prefetch Pages Command", - "- Peripheral Page Service Request", - "- X2APIC Supported", - "- NX bit Supported", - "- Guest Translation", - "- Reserved bit [5]", - "- Invalidate All Command", - "- Guest APIC supported", - "- Hardware Error Registers", - "- Performance Counters", - NULL - }; - ASSERT( iommu->mmio_base ); if ( !iommu_has_cap(iommu, PCI_CAP_EFRSUP_SHIFT) ) { - iommu->features = 0; + iommu->features.raw = 0; return; } - low = readl(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET); - high = readl(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET + 4); - - iommu->features = ((u64)high << 32) | low; + iommu->features.raw = + readq(iommu->mmio_base + IOMMU_EXT_FEATURE_MMIO_OFFSET); printk("AMD-Vi: IOMMU Extended Features:\n"); - while ( feature_str[i] ) +#define MASK(fld) ((union amd_iommu_ext_features){ .flds.fld = ~0 }).raw +#define FEAT(fld, str) do { \ + if ( MASK(fld) & (MASK(fld) - 1) ) \ + printk( "- " str ": %#x\n", iommu->features.flds.fld); \ + else if ( iommu->features.raw & MASK(fld) ) \ + printk( "- " str "\n"); \ +} while ( false ) + + FEAT(pref_sup, "Prefetch Pages Command"); + FEAT(ppr_sup, "Peripheral Page Service Request"); + FEAT(xt_sup, "x2APIC"); + FEAT(nx_sup, "NX bit"); + FEAT(gappi_sup, "Guest APIC Physical Processor Interrupt"); + FEAT(ia_sup, "Invalidate All Command"); + FEAT(ga_sup, "Guest APIC"); + FEAT(he_sup, "Hardware Error Registers"); + FEAT(pc_sup, "Performance Counters"); + FEAT(hats, "Host Address Translation Size"); + + if ( iommu->features.flds.gt_sup ) { - if ( amd_iommu_has_feature(iommu, i) ) - printk( " %s\n", feature_str[i]); - i++; + FEAT(gats, "Guest Address Translation Size"); + FEAT(glx_sup, "Guest CR3 Root Table Level"); + FEAT(pas_max, "Maximum PASID"); } + + FEAT(smif_sup, "SMI Filter Register"); + FEAT(smif_rc, "SMI Filter Register Count"); + FEAT(gam_sup, "Guest Virtual APIC Modes"); + FEAT(dual_ppr_log_sup, "Dual PPR Log"); + FEAT(dual_event_log_sup, "Dual Event Log"); + FEAT(sat_sup, "Secure ATS"); + FEAT(us_sup, "User / Supervisor Page Protection"); + FEAT(dev_tbl_seg_sup, "Device Table Segmentation"); + FEAT(ppr_early_of_sup, "PPR Log Overflow Early Warning"); + FEAT(ppr_auto_rsp_sup, "PPR Automatic Response"); + FEAT(marc_sup, "Memory Access Routing and Control"); + FEAT(blk_stop_mrk_sup, "Block StopMark Message"); + FEAT(perf_opt_sup , "Performance Optimization"); + FEAT(msi_cap_mmio_sup, "MSI Capability MMIO Access"); + FEAT(gio_sup, "Guest I/O Protection"); + FEAT(ha_sup, "Host Access"); + FEAT(eph_sup, "Enhanced PPR Handling"); + FEAT(attr_fw_sup, "Attribute Forward"); + FEAT(hd_sup, "Host Dirty"); + FEAT(inv_iotlb_type_sup, "Invalidate IOTLB Type"); + FEAT(viommu_sup, "Virtualized IOMMU"); + FEAT(vm_guard_io_sup, "VMGuard I/O Support"); + FEAT(vm_table_size, "VM Table Size"); + FEAT(ga_update_dis_sup, "Guest Access Bit Update Disable"); + +#undef FEAT +#undef MASK } int __init amd_iommu_detect_one_acpi( --- a/xen/drivers/passthrough/amd/iommu_guest.c +++ b/xen/drivers/passthrough/amd/iommu_guest.c @@ -638,7 +638,7 @@ static uint64_t iommu_mmio_read64(struct val = reg_to_u64(iommu->reg_status); break; case IOMMU_EXT_FEATURE_MMIO_OFFSET: - val = reg_to_u64(iommu->reg_ext_feature); + val = iommu->reg_ext_feature.raw; break; default: @@ -802,39 +802,26 @@ int guest_iommu_set_base(struct domain * /* Initialize mmio read only bits */ static void guest_iommu_reg_init(struct guest_iommu *iommu) { - uint32_t lower, upper; + union amd_iommu_ext_features ef = { + /* Support prefetch */ + .flds.pref_sup = 1, + /* Support PPR log */ + .flds.ppr_sup = 1, + /* Support guest translation */ + .flds.gt_sup = 1, + /* Support invalidate all command */ + .flds.ia_sup = 1, + /* Host translation size has 6 levels */ + .flds.hats = HOST_ADDRESS_SIZE_6_LEVEL, + /* Guest translation size has 6 levels */ + .flds.gats = GUEST_ADDRESS_SIZE_6_LEVEL, + /* Single level gCR3 */ + .flds.glx_sup = GUEST_CR3_1_LEVEL, + /* 9 bit PASID */ + .flds.pas_max = PASMAX_9_bit, + }; - lower = upper = 0; - /* Support prefetch */ - iommu_set_bit(&lower,IOMMU_EXT_FEATURE_PREFSUP_SHIFT); - /* Support PPR log */ - iommu_set_bit(&lower,IOMMU_EXT_FEATURE_PPRSUP_SHIFT); - /* Support guest translation */ - iommu_set_bit(&lower,IOMMU_EXT_FEATURE_GTSUP_SHIFT); - /* Support invalidate all command */ - iommu_set_bit(&lower,IOMMU_EXT_FEATURE_IASUP_SHIFT); - - /* Host translation size has 6 levels */ - set_field_in_reg_u32(HOST_ADDRESS_SIZE_6_LEVEL, lower, - IOMMU_EXT_FEATURE_HATS_MASK, - IOMMU_EXT_FEATURE_HATS_SHIFT, - &lower); - /* Guest translation size has 6 levels */ - set_field_in_reg_u32(GUEST_ADDRESS_SIZE_6_LEVEL, lower, - IOMMU_EXT_FEATURE_GATS_MASK, - IOMMU_EXT_FEATURE_GATS_SHIFT, - &lower); - /* Single level gCR3 */ - set_field_in_reg_u32(GUEST_CR3_1_LEVEL, lower, - IOMMU_EXT_FEATURE_GLXSUP_MASK, - IOMMU_EXT_FEATURE_GLXSUP_SHIFT, &lower); - /* 9 bit PASID */ - set_field_in_reg_u32(PASMAX_9_bit, upper, - IOMMU_EXT_FEATURE_PASMAX_MASK, - IOMMU_EXT_FEATURE_PASMAX_SHIFT, &upper); - - iommu->reg_ext_feature.lo = lower; - iommu->reg_ext_feature.hi = upper; + iommu->reg_ext_feature = ef; } static int guest_iommu_mmio_range(struct vcpu *v, unsigned long addr) --- a/xen/drivers/passthrough/amd/iommu_init.c +++ b/xen/drivers/passthrough/amd/iommu_init.c @@ -883,7 +883,7 @@ static void enable_iommu(struct amd_iomm register_iommu_event_log_in_mmio_space(iommu); register_iommu_exclusion_range(iommu); - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_PPRSUP_SHIFT) ) + if ( iommu->features.flds.ppr_sup ) register_iommu_ppr_log_in_mmio_space(iommu); desc = irq_to_desc(iommu->msi.irq); @@ -897,15 +897,15 @@ static void enable_iommu(struct amd_iomm set_iommu_command_buffer_control(iommu, IOMMU_CONTROL_ENABLED); set_iommu_event_log_control(iommu, IOMMU_CONTROL_ENABLED); - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_PPRSUP_SHIFT) ) + if ( iommu->features.flds.ppr_sup ) set_iommu_ppr_log_control(iommu, IOMMU_CONTROL_ENABLED); - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_GTSUP_SHIFT) ) + if ( iommu->features.flds.gt_sup ) set_iommu_guest_translation_control(iommu, IOMMU_CONTROL_ENABLED); set_iommu_translation_control(iommu, IOMMU_CONTROL_ENABLED); - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_IASUP_SHIFT) ) + if ( iommu->features.flds.ia_sup ) amd_iommu_flush_all_caches(iommu); iommu->enabled = 1; @@ -928,10 +928,10 @@ static void disable_iommu(struct amd_iom set_iommu_command_buffer_control(iommu, IOMMU_CONTROL_DISABLED); set_iommu_event_log_control(iommu, IOMMU_CONTROL_DISABLED); - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_PPRSUP_SHIFT) ) + if ( iommu->features.flds.ppr_sup ) set_iommu_ppr_log_control(iommu, IOMMU_CONTROL_DISABLED); - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_GTSUP_SHIFT) ) + if ( iommu->features.flds.gt_sup ) set_iommu_guest_translation_control(iommu, IOMMU_CONTROL_DISABLED); set_iommu_translation_control(iommu, IOMMU_CONTROL_DISABLED); @@ -1027,7 +1027,7 @@ static int __init amd_iommu_init_one(str get_iommu_features(iommu); - if ( iommu->features ) + if ( iommu->features.raw ) iommuv2_enabled = 1; if ( allocate_cmd_buffer(iommu) == NULL ) @@ -1036,9 +1036,8 @@ static int __init amd_iommu_init_one(str if ( allocate_event_log(iommu) == NULL ) goto error_out; - if ( amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_PPRSUP_SHIFT) ) - if ( allocate_ppr_log(iommu) == NULL ) - goto error_out; + if ( iommu->features.flds.ppr_sup && !allocate_ppr_log(iommu) ) + goto error_out; if ( !set_iommu_interrupt_handler(iommu) ) goto error_out; @@ -1389,7 +1388,7 @@ void amd_iommu_resume(void) } /* flush all cache entries after iommu re-enabled */ - if ( !amd_iommu_has_feature(iommu, IOMMU_EXT_FEATURE_IASUP_SHIFT) ) + if ( !iommu->features.flds.ia_sup ) { invalidate_all_devices(); invalidate_all_domain_pages(); --- a/xen/include/asm-x86/amd-iommu.h +++ b/xen/include/asm-x86/amd-iommu.h @@ -83,7 +83,7 @@ struct amd_iommu { iommu_cap_t cap; u8 ht_flags; - u64 features; + union amd_iommu_ext_features features; void *mmio_base; unsigned long mmio_base_phys; @@ -174,7 +174,7 @@ struct guest_iommu { /* MMIO regs */ struct mmio_reg reg_ctrl; /* MMIO offset 0018h */ struct mmio_reg reg_status; /* MMIO offset 2020h */ - struct mmio_reg reg_ext_feature; /* MMIO offset 0030h */ + union amd_iommu_ext_features reg_ext_feature; /* MMIO offset 0030h */ /* guest interrupt settings */ struct guest_iommu_msi msi; --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h @@ -346,26 +346,57 @@ struct amd_iommu_dte { #define IOMMU_EXCLUSION_LIMIT_HIGH_MASK 0xFFFFFFFF #define IOMMU_EXCLUSION_LIMIT_HIGH_SHIFT 0 -/* Extended Feature Register*/ +/* Extended Feature Register */ #define IOMMU_EXT_FEATURE_MMIO_OFFSET 0x30 -#define IOMMU_EXT_FEATURE_PREFSUP_SHIFT 0x0 -#define IOMMU_EXT_FEATURE_PPRSUP_SHIFT 0x1 -#define IOMMU_EXT_FEATURE_XTSUP_SHIFT 0x2 -#define IOMMU_EXT_FEATURE_NXSUP_SHIFT 0x3 -#define IOMMU_EXT_FEATURE_GTSUP_SHIFT 0x4 -#define IOMMU_EXT_FEATURE_IASUP_SHIFT 0x6 -#define IOMMU_EXT_FEATURE_GASUP_SHIFT 0x7 -#define IOMMU_EXT_FEATURE_HESUP_SHIFT 0x8 -#define IOMMU_EXT_FEATURE_PCSUP_SHIFT 0x9 -#define IOMMU_EXT_FEATURE_HATS_SHIFT 0x10 -#define IOMMU_EXT_FEATURE_HATS_MASK 0x00000C00 -#define IOMMU_EXT_FEATURE_GATS_SHIFT 0x12 -#define IOMMU_EXT_FEATURE_GATS_MASK 0x00003000 -#define IOMMU_EXT_FEATURE_GLXSUP_SHIFT 0x14 -#define IOMMU_EXT_FEATURE_GLXSUP_MASK 0x0000C000 -#define IOMMU_EXT_FEATURE_PASMAX_SHIFT 0x0 -#define IOMMU_EXT_FEATURE_PASMAX_MASK 0x0000001F +union amd_iommu_ext_features { + uint64_t raw; + struct { + unsigned int pref_sup:1; + unsigned int ppr_sup:1; + unsigned int xt_sup:1; + unsigned int nx_sup:1; + unsigned int gt_sup:1; + unsigned int gappi_sup:1; + unsigned int ia_sup:1; + unsigned int ga_sup:1; + unsigned int he_sup:1; + unsigned int pc_sup:1; + unsigned int hats:2; + unsigned int gats:2; + unsigned int glx_sup:2; + unsigned int smif_sup:2; + unsigned int smif_rc:3; + unsigned int gam_sup:3; + unsigned int dual_ppr_log_sup:2; + unsigned int :2; + unsigned int dual_event_log_sup:2; + unsigned int sat_sup:1; + unsigned int :1; + unsigned int pas_max:5; + unsigned int us_sup:1; + unsigned int dev_tbl_seg_sup:2; + unsigned int ppr_early_of_sup:1; + unsigned int ppr_auto_rsp_sup:1; + unsigned int marc_sup:2; + unsigned int blk_stop_mrk_sup:1; + unsigned int perf_opt_sup:1; + unsigned int msi_cap_mmio_sup:1; + unsigned int :1; + unsigned int gio_sup:1; + unsigned int ha_sup:1; + unsigned int eph_sup:1; + unsigned int attr_fw_sup:1; + unsigned int hd_sup:1; + unsigned int :1; + unsigned int inv_iotlb_type_sup:1; + unsigned int viommu_sup:1; + unsigned int vm_guard_io_sup:1; + unsigned int vm_table_size:4; + unsigned int ga_update_dis_sup:1; + unsigned int :2; + } flds; +}; /* Status Register*/ #define IOMMU_STATUS_MMIO_OFFSET 0x2020 --- a/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-proto.h @@ -219,13 +219,6 @@ static inline int iommu_has_cap(struct a return !!(iommu->cap.header & (1u << bit)); } -static inline int amd_iommu_has_feature(struct amd_iommu *iommu, uint32_t bit) -{ - if ( !iommu_has_cap(iommu, PCI_CAP_EFRSUP_SHIFT) ) - return 0; - return !!(iommu->features & (1U << bit)); -} - /* access tail or head pointer of ring buffer */ static inline uint32_t iommu_get_rb_pointer(uint32_t reg) {
This also takes care of several of the shift values wrongly having been specified as hex rather than dec. Take the opportunity and add further fields. Signed-off-by: Jan Beulich <jbeulich@suse.com>