From: "Tian, Kevin" <kevin.tian@intel.com>
To: Paul Durrant <paul@xen.org>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: Paul Durrant <pdurrant@amazon.com>
Subject: RE: [PATCH v10 6/7] vtd: use a bit field for context_entry
Date: Mon, 30 Nov 2020 03:10:33 +0000 [thread overview]
Message-ID: <MWHPR11MB16457F22A1940E866803819D8CF50@MWHPR11MB1645.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20201120132440.1141-7-paul@xen.org>
> From: Paul Durrant <paul@xen.org>
> Sent: Friday, November 20, 2020 9:25 PM
>
> From: Paul Durrant <pdurrant@amazon.com>
>
> This removes the need for much shifting, masking and several magic
> numbers.
> On the whole it makes the code quite a bit more readable.
>
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
> Cc: Kevin Tian <kevin.tian@intel.com>
>
> v10:
> - Remove macros in favour of direct field access
> - Adjust field types
> - Add missing barriers
>
> v4:
> - New in v4
> ---
> xen/drivers/passthrough/vtd/iommu.c | 36 +++++++++++----------
> xen/drivers/passthrough/vtd/iommu.h | 45 +++++++++++++--------------
> xen/drivers/passthrough/vtd/utils.c | 10 +++---
> xen/drivers/passthrough/vtd/x86/ats.c | 6 ++--
> 4 files changed, 47 insertions(+), 50 deletions(-)
>
> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 1a038541f0a3..fdb472ad6515 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -86,8 +86,6 @@ static int domain_iommu_domid(struct domain *d,
> return -1;
> }
>
> -#define DID_FIELD_WIDTH 16
> -#define DID_HIGH_OFFSET 8
> static int context_set_domain_id(struct context_entry *context,
> struct domain *d,
> struct vtd_iommu *iommu)
> @@ -121,21 +119,22 @@ static int context_set_domain_id(struct
> context_entry *context,
> }
>
> set_bit(i, iommu->domid_bitmap);
> - context->hi |= (i & ((1 << DID_FIELD_WIDTH) - 1)) << DID_HIGH_OFFSET;
> + context->did = i;
> +
> return 0;
> }
>
> static int context_get_domain_id(struct context_entry *context,
> struct vtd_iommu *iommu)
> {
> - unsigned long dom_index, nr_dom;
> int domid = -1;
>
> if (iommu && context)
> {
> - nr_dom = cap_ndoms(iommu->cap);
> + unsigned long dom_index, nr_dom;
>
> - dom_index = context_domain_id(*context);
> + nr_dom = cap_ndoms(iommu->cap);
> + dom_index = context->did;
>
> if ( dom_index < nr_dom && iommu->domid_map )
> domid = iommu->domid_map[dom_index];
> @@ -1338,7 +1337,7 @@ int domain_context_mapping_one(
> context_entries = (struct context_entry *)map_vtd_domain_page(maddr);
> context = &context_entries[devfn];
>
> - if ( context_present(*context) )
> + if ( context->p )
> {
> int res = 0;
>
> @@ -1382,7 +1381,7 @@ int domain_context_mapping_one(
>
> if ( iommu_hwdom_passthrough && is_hardware_domain(domain) )
> {
> - context_set_translation_type(*context, CONTEXT_TT_PASS_THRU);
> + context->tt = CONTEXT_TT_PASS_THRU;
> }
> else
> {
> @@ -1397,11 +1396,11 @@ int domain_context_mapping_one(
> return -ENOMEM;
> }
>
> - context_set_address_root(*context, pgd_maddr);
> + context->slptptr = paddr_to_pfn(pgd_maddr);
> if ( ats_enabled && ecap_dev_iotlb(iommu->ecap) )
> - context_set_translation_type(*context, CONTEXT_TT_DEV_IOTLB);
> + context->tt = CONTEXT_TT_DEV_IOTLB;
> else
> - context_set_translation_type(*context, CONTEXT_TT_MULTI_LEVEL);
> + context->tt = CONTEXT_TT_MULTI_LEVEL;
>
> spin_unlock(&hd->arch.mapping_lock);
> }
> @@ -1413,9 +1412,10 @@ int domain_context_mapping_one(
> return -EFAULT;
> }
>
> - context_set_address_width(*context, level_to_agaw(iommu-
> >nr_pt_levels));
> - context_set_fault_enable(*context);
> - context_set_present(*context);
> + context->aw = level_to_agaw(iommu->nr_pt_levels);
> + context->fpd = false;
> + smp_wmb();
> + context->p = true;
> iommu_sync_cache(context, sizeof(struct context_entry));
> spin_unlock(&iommu->lock);
>
> @@ -1567,17 +1567,19 @@ int domain_context_unmap_one(
> context_entries = (struct context_entry *)map_vtd_domain_page(maddr);
> context = &context_entries[devfn];
>
> - if ( !context_present(*context) )
> + if ( !context->p )
> {
> spin_unlock(&iommu->lock);
> unmap_vtd_domain_page(context_entries);
> return 0;
> }
>
> - context_clear_present(*context);
> - context_clear_entry(*context);
> + context->p = false;
> + smp_wmb();
> iommu_sync_cache(context, sizeof(struct context_entry));
>
> + context->val = 0; /* No need to sync; present bit is already cleared */
> +
> iommu_domid= domain_iommu_domid(domain, iommu);
> if ( iommu_domid == -1 )
> {
> diff --git a/xen/drivers/passthrough/vtd/iommu.h
> b/xen/drivers/passthrough/vtd/iommu.h
> index b14628eec260..33b1abf98526 100644
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -198,37 +198,34 @@ struct root_entry {
> };
> };
> };
> +#define ROOT_ENTRY_NR (PAGE_SIZE_4K / sizeof(struct root_entry))
>
> struct context_entry {
> - u64 lo;
> - u64 hi;
> -};
> -#define ROOT_ENTRY_NR (PAGE_SIZE_4K/sizeof(struct root_entry))
> -#define context_present(c) ((c).lo & 1)
> -#define context_fault_disable(c) (((c).lo >> 1) & 1)
> -#define context_translation_type(c) (((c).lo >> 2) & 3)
> -#define context_address_root(c) ((c).lo & PAGE_MASK_4K)
> -#define context_address_width(c) ((c).hi & 7)
> -#define context_domain_id(c) (((c).hi >> 8) & ((1 << 16) - 1))
> + union {
> + __uint128_t val;
> + struct { uint64_t lo, hi; };
> + struct {
> + /* 0 - 63 */
> + bool p:1;
> + bool fpd:1;
> + uint64_t tt:2;
>
> -#define context_set_present(c) do {(c).lo |= 1;} while(0)
> -#define context_clear_present(c) do {(c).lo &= ~1;} while(0)
> -#define context_set_fault_enable(c) \
> - do {(c).lo &= (((u64)-1) << 2) | 1;} while(0)
> -
> -#define context_set_translation_type(c, val) do { \
> - (c).lo &= (((u64)-1) << 4) | 3; \
> - (c).lo |= (val & 3) << 2; \
> - } while(0)
> #define CONTEXT_TT_MULTI_LEVEL 0
> #define CONTEXT_TT_DEV_IOTLB 1
> #define CONTEXT_TT_PASS_THRU 2
>
> -#define context_set_address_root(c, val) \
> - do {(c).lo &= 0xfff; (c).lo |= (val) & PAGE_MASK_4K ;} while(0)
> -#define context_set_address_width(c, val) \
> - do {(c).hi &= 0xfffffff8; (c).hi |= (val) & 7;} while(0)
> -#define context_clear_entry(c) do {(c).lo = 0; (c).hi = 0;} while(0)
> + unsigned int reserved0:8;
> + uint64_t slptptr:52;
> +
> + /* 64 - 127 */
> + unsigned int aw:3;
> + unsigned int ignored:4;
> + unsigned int reserved1:1;
> + unsigned int did:16;
> + uint64_t reserved2:40;
> + };
> + };
> +};
>
> /* page table handling */
> #define LEVEL_STRIDE (9)
> diff --git a/xen/drivers/passthrough/vtd/utils.c
> b/xen/drivers/passthrough/vtd/utils.c
> index 5f25a86a535c..4bca160bc663 100644
> --- a/xen/drivers/passthrough/vtd/utils.c
> +++ b/xen/drivers/passthrough/vtd/utils.c
> @@ -129,17 +129,17 @@ void print_vtd_entries(struct vtd_iommu *iommu,
> int bus, int devfn, u64 gmfn)
> return;
> }
>
> - val = ctxt_entry[devfn].lo;
> - printk(" context[%02x] = %"PRIx64"_%"PRIx64"\n",
> - devfn, ctxt_entry[devfn].hi, val);
> - if ( !context_present(ctxt_entry[devfn]) )
> + printk(" context[%02x] = %"PRIx64"_%"PRIx64"\n", devfn,
> + ctxt_entry[devfn].hi, ctxt_entry[devfn].lo);
> + if ( !ctxt_entry[devfn].p )
> {
> unmap_vtd_domain_page(ctxt_entry);
> printk(" ctxt_entry[%02x] not present\n", devfn);
> return;
> }
>
> - level = agaw_to_level(context_address_width(ctxt_entry[devfn]));
> + level = agaw_to_level(ctxt_entry[devfn].aw);
> + val = pfn_to_paddr(ctxt_entry[devfn].slptptr);
> unmap_vtd_domain_page(ctxt_entry);
> if ( level != VTD_PAGE_TABLE_LEVEL_3 &&
> level != VTD_PAGE_TABLE_LEVEL_4)
> diff --git a/xen/drivers/passthrough/vtd/x86/ats.c
> b/xen/drivers/passthrough/vtd/x86/ats.c
> index fec969ef75bb..cb057ced3cf7 100644
> --- a/xen/drivers/passthrough/vtd/x86/ats.c
> +++ b/xen/drivers/passthrough/vtd/x86/ats.c
> @@ -76,7 +76,6 @@ static bool device_in_domain(const struct vtd_iommu
> *iommu,
> {
> struct root_entry *root_entry, *root_entries;
> struct context_entry *context_entry, *context_entries = NULL;
> - unsigned int tt;
> bool found = false;
>
> if ( unlikely(!iommu->root_maddr) )
> @@ -92,11 +91,10 @@ static bool device_in_domain(const struct
> vtd_iommu *iommu,
>
> context_entries = map_vtd_domain_page(root_entry->ctp);
> context_entry = &context_entries[pdev->devfn];
> - if ( context_domain_id(*context_entry) != did )
> + if ( context_entry->did != did )
> goto out;
>
> - tt = context_translation_type(*context_entry);
> - if ( tt != CONTEXT_TT_DEV_IOTLB )
> + if ( context_entry->tt != CONTEXT_TT_DEV_IOTLB )
> goto out;
>
> found = true;
> --
> 2.20.1
next prev parent reply other threads:[~2020-11-30 3:11 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-20 13:24 [PATCH v10 0/7] IOMMU cleanup Paul Durrant
2020-11-20 13:24 ` [PATCH v10 1/7] remove remaining uses of iommu_legacy_map/unmap Paul Durrant
2020-11-27 14:39 ` Jan Beulich
2020-11-20 13:24 ` [PATCH v10 2/7] common/grant_table: batch flush I/O TLB Paul Durrant
2020-11-20 13:24 ` [PATCH v10 3/7] iommu: remove the share_p2m operation Paul Durrant
2020-11-20 13:24 ` [PATCH v10 4/7] iommu: stop calling IOMMU page tables 'p2m tables' Paul Durrant
2020-11-20 13:24 ` [PATCH v10 5/7] vtd: use a bit field for root_entry Paul Durrant
2020-11-27 15:11 ` Jan Beulich
2020-11-30 3:06 ` Tian, Kevin
2020-11-30 9:45 ` Jan Beulich
2020-12-01 5:14 ` Tian, Kevin
2020-11-20 13:24 ` [PATCH v10 6/7] vtd: use a bit field for context_entry Paul Durrant
2020-11-27 15:32 ` Jan Beulich
2020-11-30 3:10 ` Tian, Kevin [this message]
2020-11-20 13:24 ` [PATCH v10 7/7] vtd: use a bit field for dma_pte Paul Durrant
2020-11-27 16:02 ` Jan Beulich
2020-11-30 5:29 ` Tian, Kevin
2020-11-27 16:21 ` [PATCH v10 0/7] IOMMU cleanup Jan Beulich
2020-11-27 16:32 ` Durrant, Paul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR11MB16457F22A1940E866803819D8CF50@MWHPR11MB1645.namprd11.prod.outlook.com \
--to=kevin.tian@intel.com \
--cc=paul@xen.org \
--cc=pdurrant@amazon.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.