From: "Michael S. Tsirkin" <mst@redhat.com> To: qemu-devel@nongnu.org Cc: Peter Maydell <peter.maydell@linaro.org>, Jason Wang <jasowang@redhat.com>, Peter Xu <peterx@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Subject: [PULL 23/31] intel_iommu: refine iotlb hash calculation Date: Tue, 25 Apr 2023 03:46:12 -0400 [thread overview] Message-ID: <ec1a78cee97001b0ed25b5866e92dae058eb5877.1682408661.git.mst@redhat.com> (raw) In-Reply-To: <cover.1682408661.git.mst@redhat.com> From: Jason Wang <jasowang@redhat.com> Commit 1b2b12376c8 ("intel-iommu: PASID support") takes PASID into account when calculating iotlb hash like: static guint vtd_iotlb_hash(gconstpointer v) { const struct vtd_iotlb_key *key = v; return key->gfn | ((key->sid) << VTD_IOTLB_SID_SHIFT) | (key->level) << VTD_IOTLB_LVL_SHIFT | (key->pasid) << VTD_IOTLB_PASID_SHIFT; } This turns out to be problematic since: - the shift will lose bits if not converting to uint64_t - level should be off by one in order to fit into 2 bits - VTD_IOTLB_PASID_SHIFT is 30 but PASID is 20 bits which will waste some bits - the hash result is uint64_t so we will lose bits when converting to guint So this patch fixes them by - converting the keys into uint64_t before doing the shift - off level by one to make it fit into two bits - change the sid, lvl and pasid shift to 26, 42 and 44 in order to take the full width of uint64_t - perform an XOR to the top 32bit with the bottom 32bit for the final result to fit guint Fixes: Coverity CID 1508100 Fixes: 1b2b12376c8 ("intel-iommu: PASID support") Signed-off-by: Jason Wang <jasowang@redhat.com> Message-Id: <20230412073510.7158-1-jasowang@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> --- hw/i386/intel_iommu_internal.h | 6 +++--- hw/i386/intel_iommu.c | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h index f090e61e11..2e61eec2f5 100644 --- a/hw/i386/intel_iommu_internal.h +++ b/hw/i386/intel_iommu_internal.h @@ -114,9 +114,9 @@ VTD_INTERRUPT_ADDR_FIRST + 1) /* The shift of source_id in the key of IOTLB hash table */ -#define VTD_IOTLB_SID_SHIFT 20 -#define VTD_IOTLB_LVL_SHIFT 28 -#define VTD_IOTLB_PASID_SHIFT 30 +#define VTD_IOTLB_SID_SHIFT 26 +#define VTD_IOTLB_LVL_SHIFT 42 +#define VTD_IOTLB_PASID_SHIFT 44 #define VTD_IOTLB_MAX_SIZE 1024 /* Max size of the hash table */ /* IOTLB_REG */ diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index a62896759c..94d52f4205 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -64,8 +64,8 @@ struct vtd_as_key { struct vtd_iotlb_key { uint64_t gfn; uint32_t pasid; - uint32_t level; uint16_t sid; + uint8_t level; }; static void vtd_address_space_refresh_all(IntelIOMMUState *s); @@ -221,10 +221,11 @@ static gboolean vtd_iotlb_equal(gconstpointer v1, gconstpointer v2) static guint vtd_iotlb_hash(gconstpointer v) { const struct vtd_iotlb_key *key = v; + uint64_t hash64 = key->gfn | ((uint64_t)(key->sid) << VTD_IOTLB_SID_SHIFT) | + (uint64_t)(key->level - 1) << VTD_IOTLB_LVL_SHIFT | + (uint64_t)(key->pasid) << VTD_IOTLB_PASID_SHIFT; - return key->gfn | ((key->sid) << VTD_IOTLB_SID_SHIFT) | - (key->level) << VTD_IOTLB_LVL_SHIFT | - (key->pasid) << VTD_IOTLB_PASID_SHIFT; + return (guint)((hash64 >> 32) ^ (hash64 & 0xffffffffU)); } static gboolean vtd_as_equal(gconstpointer v1, gconstpointer v2) -- MST
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com> To: qemu-devel@nongnu.org Cc: Peter Maydell <peter.maydell@linaro.org>, Jason Wang <jasowang@redhat.com>, Peter Xu <peterx@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com> Subject: [PULL 23/31] intel_iommu: refine iotlb hash calculation Date: Tue, 25 Apr 2023 04:04:15 -0400 [thread overview] Message-ID: <ec1a78cee97001b0ed25b5866e92dae058eb5877.1682408661.git.mst@redhat.com> (raw) Message-ID: <20230425080415.R9cjwZv06nDeZ0PoGCKqC6u8vV9Zjx2uAZ77bIUAaW0@z> (raw) In-Reply-To: <cover.1682408661.git.mst@redhat.com> From: Jason Wang <jasowang@redhat.com> Commit 1b2b12376c8 ("intel-iommu: PASID support") takes PASID into account when calculating iotlb hash like: static guint vtd_iotlb_hash(gconstpointer v) { const struct vtd_iotlb_key *key = v; return key->gfn | ((key->sid) << VTD_IOTLB_SID_SHIFT) | (key->level) << VTD_IOTLB_LVL_SHIFT | (key->pasid) << VTD_IOTLB_PASID_SHIFT; } This turns out to be problematic since: - the shift will lose bits if not converting to uint64_t - level should be off by one in order to fit into 2 bits - VTD_IOTLB_PASID_SHIFT is 30 but PASID is 20 bits which will waste some bits - the hash result is uint64_t so we will lose bits when converting to guint So this patch fixes them by - converting the keys into uint64_t before doing the shift - off level by one to make it fit into two bits - change the sid, lvl and pasid shift to 26, 42 and 44 in order to take the full width of uint64_t - perform an XOR to the top 32bit with the bottom 32bit for the final result to fit guint Fixes: Coverity CID 1508100 Fixes: 1b2b12376c8 ("intel-iommu: PASID support") Signed-off-by: Jason Wang <jasowang@redhat.com> Message-Id: <20230412073510.7158-1-jasowang@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Peter Xu <peterx@redhat.com> --- hw/i386/intel_iommu_internal.h | 6 +++--- hw/i386/intel_iommu.c | 9 +++++---- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h index f090e61e11..2e61eec2f5 100644 --- a/hw/i386/intel_iommu_internal.h +++ b/hw/i386/intel_iommu_internal.h @@ -114,9 +114,9 @@ VTD_INTERRUPT_ADDR_FIRST + 1) /* The shift of source_id in the key of IOTLB hash table */ -#define VTD_IOTLB_SID_SHIFT 20 -#define VTD_IOTLB_LVL_SHIFT 28 -#define VTD_IOTLB_PASID_SHIFT 30 +#define VTD_IOTLB_SID_SHIFT 26 +#define VTD_IOTLB_LVL_SHIFT 42 +#define VTD_IOTLB_PASID_SHIFT 44 #define VTD_IOTLB_MAX_SIZE 1024 /* Max size of the hash table */ /* IOTLB_REG */ diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index a62896759c..94d52f4205 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -64,8 +64,8 @@ struct vtd_as_key { struct vtd_iotlb_key { uint64_t gfn; uint32_t pasid; - uint32_t level; uint16_t sid; + uint8_t level; }; static void vtd_address_space_refresh_all(IntelIOMMUState *s); @@ -221,10 +221,11 @@ static gboolean vtd_iotlb_equal(gconstpointer v1, gconstpointer v2) static guint vtd_iotlb_hash(gconstpointer v) { const struct vtd_iotlb_key *key = v; + uint64_t hash64 = key->gfn | ((uint64_t)(key->sid) << VTD_IOTLB_SID_SHIFT) | + (uint64_t)(key->level - 1) << VTD_IOTLB_LVL_SHIFT | + (uint64_t)(key->pasid) << VTD_IOTLB_PASID_SHIFT; - return key->gfn | ((key->sid) << VTD_IOTLB_SID_SHIFT) | - (key->level) << VTD_IOTLB_LVL_SHIFT | - (key->pasid) << VTD_IOTLB_PASID_SHIFT; + return (guint)((hash64 >> 32) ^ (hash64 & 0xffffffffU)); } static gboolean vtd_as_equal(gconstpointer v1, gconstpointer v2) -- MST
next prev parent reply other threads:[~2023-04-25 7:50 UTC|newest] Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-04-25 7:44 [PULL 00/31] virtio,pc,pci: fixes, features, cleanups Michael S. Tsirkin 2023-04-25 7:44 ` [PULL 01/31] virtio: refresh vring region cache after updating a virtqueue size Michael S. Tsirkin 2023-04-26 16:32 ` Michael Tokarev 2023-04-25 7:44 ` [PULL 02/31] Add my old and new work email mapping and use work email to support biosbits Michael S. Tsirkin 2023-04-25 7:44 ` [PULL 03/31] vdpa: accept VIRTIO_NET_F_SPEED_DUPLEX in SVQ Michael S. Tsirkin 2023-04-25 7:44 ` [PULL 04/31] meson_options.txt: Enable qom-cast-debug by default again Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 05/31] vhost: Drop unused eventfd_add|del hooks Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 06/31] docs: vhost-user: Define memory region separately Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 07/31] docs: vhost-user: Add Xen specific memory mapping support Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 08/31] virtio-balloon: optimize the virtio-balloon on the ARM platform Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 09/31] MAINTAINERS: Mark AMD-Vi emulation as orphan Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 10/31] hw/i386/amd_iommu: Explicit use of AMDVI_BASE_ADDR in amdvi_init Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 11/31] hw/i386/amd_iommu: Remove intermediate AMDVIState::devid field Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 12/31] hw/i386/amd_iommu: Move capab_offset from AMDVIState to AMDVIPCIState Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 13/31] hw/i386/amd_iommu: Set PCI static/const fields via PCIDeviceClass Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 14/31] hw/i386/amd_iommu: Factor amdvi_pci_realize out of amdvi_sysbus_realize Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 15/31] hw: Add compat machines for 8.1 Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 16/31] pci: avoid accessing slot_reserved_mask directly outside of pci.c Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 17/31] vhost-user-blk-server: notify client about disk resize Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 18/31] Add my old and new work email mapping and use work email to support acpi Michael S. Tsirkin 2023-04-25 8:22 ` Ani Sinha 2023-04-25 7:45 ` [PULL 19/31] hw/acpi: limit warning on acpi table size to pc machines older than version 2.3 Michael S. Tsirkin 2023-04-25 7:45 ` [PULL 20/31] tests: bios-tables-test: replace memset with initializer Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 21/31] MAINTAINERS: Add Eugenio Pérez as vhost-shadow-virtqueue reviewer Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 22/31] docs/cxl: Fix sentence Michael S. Tsirkin 2023-04-25 7:46 ` Michael S. Tsirkin [this message] 2023-04-25 8:04 ` [PULL 23/31] intel_iommu: refine iotlb hash calculation Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 25/31] virtio: i2c: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 26/31] acpi: pcihp: allow repeating hot-unplug requests Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 27/31] docs/specs/pci-ids: Convert from txt to rST Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 28/31] docs/specs: Convert pci-serial.txt to rst Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 29/31] docs/specs: Convert pci-testdev.txt " Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 30/31] hw/pci-bridge: pci_expander_bridge fix type in pxb_cxl_dev_reset() Michael S. Tsirkin 2023-04-25 7:46 ` [PULL 31/31] hw/pci-bridge: Make PCIe and CXL PXB Devices inherit from TYPE_PXB_DEV Michael S. Tsirkin 2023-04-25 8:04 ` [PULL 24/31] docs: Remove obsolete descriptions of SR-IOV support Michael S. Tsirkin 2023-04-25 11:16 ` [PULL 00/31] virtio,pc,pci: fixes, features, cleanups Richard Henderson
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=ec1a78cee97001b0ed25b5866e92dae058eb5877.1682408661.git.mst@redhat.com \ --to=mst@redhat.com \ --cc=eduardo@habkost.net \ --cc=jasowang@redhat.com \ --cc=marcel.apfelbaum@gmail.com \ --cc=pbonzini@redhat.com \ --cc=peter.maydell@linaro.org \ --cc=peterx@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=richard.henderson@linaro.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).