|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 01/12] AMD/IOMMU: use bit field for extended feature register
On Thu, Jul 25, 2019 at 01:29:16PM +0000, 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
> - replace a readl() pair by a single readq(),
> - add further fields.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
Acked-by: Brian Woods <brian.woods@xxxxxxx>
> ---
> v4: Drop stray/leftover #undef.
> v3: Another attempt at deriving masks from bitfields, hopefully better
> liked by clang (mine was fine even with the v2 variant).
> v2: Correct sats_sup position and name. Re-base over new earlier patch.
>
> --- a/xen/drivers/passthrough/amd/iommu_detect.c
> +++ b/xen/drivers/passthrough/amd/iommu_detect.c
> @@ -60,49 +60,76 @@ static int __init get_iommu_capabilities
>
> void __init get_iommu_features(struct amd_iommu *iommu)
> {
> - u32 low, high;
> - int i = 0 ;
> const struct amd_iommu *first;
> - 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);
>
> /* Don't log the same set of features over and over. */
> first = list_first_entry(&amd_iommu_head, struct amd_iommu, list);
> - if ( iommu != first && iommu->features == first->features )
> + if ( iommu != first && iommu->features.raw == first->features.raw )
> return;
>
> printk("AMD-Vi: IOMMU Extended Features:\n");
>
> - while ( feature_str[i] )
> +#define FEAT(fld, str) do { \
> + if ( --((union amd_iommu_ext_features){}).flds.fld > 1 ) \
> + printk( "- " str ": %#x\n", iommu->features.flds.fld); \
> + else if ( iommu->features.flds.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(sats_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
> }
>
> 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
> @@ -882,7 +882,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);
> @@ -896,15 +896,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;
> @@ -927,10 +927,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);
> @@ -1026,7 +1026,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 )
> @@ -1035,9 +1035,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;
> @@ -1393,7 +1392,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;
> @@ -175,7 +175,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 :1;
> + unsigned int sats_sup: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
> @@ -218,13 +218,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)
> {
>
--
Brian Woods
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |