| Message ID | 20240930092631.2997543-12-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | intel_iommu: Enable stage-1 translation for emulated device | expand |
On 2024/9/30 17:26, Zhenzhong Duan wrote: > From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> > > This will be used to implement the device IOTLB invalidation > > Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > Acked-by: Jason Wang <jasowang@redhat.com> > --- > hw/i386/intel_iommu.c | 39 ++++++++++++++++++++++++--------------- > 1 file changed, 24 insertions(+), 15 deletions(-) > > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > index 289278ce30..a1596ba47d 100644 > --- a/hw/i386/intel_iommu.c > +++ b/hw/i386/intel_iommu.c > @@ -70,6 +70,11 @@ struct vtd_hiod_key { > uint8_t devfn; > }; > > +struct vtd_as_raw_key { > + uint16_t sid; > + uint32_t pasid; > +}; > + > struct vtd_iotlb_key { > uint64_t gfn; > uint32_t pasid; > @@ -1875,29 +1880,33 @@ static inline bool vtd_is_interrupt_addr(hwaddr addr) > return VTD_INTERRUPT_ADDR_FIRST <= addr && addr <= VTD_INTERRUPT_ADDR_LAST; > } > > -static gboolean vtd_find_as_by_sid(gpointer key, gpointer value, > - gpointer user_data) > +static gboolean vtd_find_as_by_sid_and_pasid(gpointer key, gpointer value, > + gpointer user_data) > { > struct vtd_as_key *as_key = (struct vtd_as_key *)key; > - uint16_t target_sid = *(uint16_t *)user_data; > + struct vtd_as_raw_key target = *(struct vtd_as_raw_key *)user_data; why not just define target as a pointer? > uint16_t sid = PCI_BUILD_BDF(pci_bus_num(as_key->bus), as_key->devfn); > - return sid == target_sid; > + > + return (as_key->pasid == target.pasid) && > + (sid == target.sid); hence using target->pasid and target->sid here. Otherwise, looks good to me. Reviewed-by: Yi Liu <yi.l.liu@intel.com> > } > > -static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid) > +static VTDAddressSpace *vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s, > + uint16_t sid, > + uint32_t pasid) > { > - uint8_t bus_num = PCI_BUS_NUM(sid); > - VTDAddressSpace *vtd_as = s->vtd_as_cache[bus_num]; > - > - if (vtd_as && > - (sid == PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn))) { > - return vtd_as; > - } > + struct vtd_as_raw_key key = { > + .sid = sid, > + .pasid = pasid > + }; > > - vtd_as = g_hash_table_find(s->vtd_address_spaces, vtd_find_as_by_sid, &sid); > - s->vtd_as_cache[bus_num] = vtd_as; > + return g_hash_table_find(s->vtd_address_spaces, > + vtd_find_as_by_sid_and_pasid, &key); > +} > > - return vtd_as; > +static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid) > +{ > + return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID); > } > > static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)
>-----Original Message----- >From: Liu, Yi L <yi.l.liu@intel.com> >Sent: Monday, November 4, 2024 10:51 AM >Subject: Re: [PATCH v4 11/17] intel_iommu: Add an internal API to find an address >space with PASID > >On 2024/9/30 17:26, Zhenzhong Duan wrote: >> From: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> >> >> This will be used to implement the device IOTLB invalidation >> >> Signed-off-by: Clément Mathieu--Drif <clement.mathieu--drif@eviden.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> >> Acked-by: Jason Wang <jasowang@redhat.com> >> --- >> hw/i386/intel_iommu.c | 39 ++++++++++++++++++++++++--------------- >> 1 file changed, 24 insertions(+), 15 deletions(-) >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c >> index 289278ce30..a1596ba47d 100644 >> --- a/hw/i386/intel_iommu.c >> +++ b/hw/i386/intel_iommu.c >> @@ -70,6 +70,11 @@ struct vtd_hiod_key { >> uint8_t devfn; >> }; >> >> +struct vtd_as_raw_key { >> + uint16_t sid; >> + uint32_t pasid; >> +}; >> + >> struct vtd_iotlb_key { >> uint64_t gfn; >> uint32_t pasid; >> @@ -1875,29 +1880,33 @@ static inline bool vtd_is_interrupt_addr(hwaddr >addr) >> return VTD_INTERRUPT_ADDR_FIRST <= addr && addr <= >VTD_INTERRUPT_ADDR_LAST; >> } >> >> -static gboolean vtd_find_as_by_sid(gpointer key, gpointer value, >> - gpointer user_data) >> +static gboolean vtd_find_as_by_sid_and_pasid(gpointer key, gpointer value, >> + gpointer user_data) >> { >> struct vtd_as_key *as_key = (struct vtd_as_key *)key; >> - uint16_t target_sid = *(uint16_t *)user_data; >> + struct vtd_as_raw_key target = *(struct vtd_as_raw_key *)user_data; > >why not just define target as a pointer? > >> uint16_t sid = PCI_BUILD_BDF(pci_bus_num(as_key->bus), as_key->devfn); >> - return sid == target_sid; >> + >> + return (as_key->pasid == target.pasid) && >> + (sid == target.sid); > >hence using target->pasid and target->sid here. Sure, will do. Thanks Zhenzhong > Otherwise, looks good to me. > >Reviewed-by: Yi Liu <yi.l.liu@intel.com> > >> } >> >> -static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid) >> +static VTDAddressSpace *vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s, >> + uint16_t sid, >> + uint32_t pasid) >> { >> - uint8_t bus_num = PCI_BUS_NUM(sid); >> - VTDAddressSpace *vtd_as = s->vtd_as_cache[bus_num]; >> - >> - if (vtd_as && >> - (sid == PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn))) { >> - return vtd_as; >> - } >> + struct vtd_as_raw_key key = { >> + .sid = sid, >> + .pasid = pasid >> + }; >> >> - vtd_as = g_hash_table_find(s->vtd_address_spaces, vtd_find_as_by_sid, >&sid); >> - s->vtd_as_cache[bus_num] = vtd_as; >> + return g_hash_table_find(s->vtd_address_spaces, >> + vtd_find_as_by_sid_and_pasid, &key); >> +} >> >> - return vtd_as; >> +static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid) >> +{ >> + return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID); >> } >> >> static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id) > >-- >Regards, >Yi Liu
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c index 289278ce30..a1596ba47d 100644 --- a/hw/i386/intel_iommu.c +++ b/hw/i386/intel_iommu.c @@ -70,6 +70,11 @@ struct vtd_hiod_key { uint8_t devfn; }; +struct vtd_as_raw_key { + uint16_t sid; + uint32_t pasid; +}; + struct vtd_iotlb_key { uint64_t gfn; uint32_t pasid; @@ -1875,29 +1880,33 @@ static inline bool vtd_is_interrupt_addr(hwaddr addr) return VTD_INTERRUPT_ADDR_FIRST <= addr && addr <= VTD_INTERRUPT_ADDR_LAST; } -static gboolean vtd_find_as_by_sid(gpointer key, gpointer value, - gpointer user_data) +static gboolean vtd_find_as_by_sid_and_pasid(gpointer key, gpointer value, + gpointer user_data) { struct vtd_as_key *as_key = (struct vtd_as_key *)key; - uint16_t target_sid = *(uint16_t *)user_data; + struct vtd_as_raw_key target = *(struct vtd_as_raw_key *)user_data; uint16_t sid = PCI_BUILD_BDF(pci_bus_num(as_key->bus), as_key->devfn); - return sid == target_sid; + + return (as_key->pasid == target.pasid) && + (sid == target.sid); } -static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid) +static VTDAddressSpace *vtd_get_as_by_sid_and_pasid(IntelIOMMUState *s, + uint16_t sid, + uint32_t pasid) { - uint8_t bus_num = PCI_BUS_NUM(sid); - VTDAddressSpace *vtd_as = s->vtd_as_cache[bus_num]; - - if (vtd_as && - (sid == PCI_BUILD_BDF(pci_bus_num(vtd_as->bus), vtd_as->devfn))) { - return vtd_as; - } + struct vtd_as_raw_key key = { + .sid = sid, + .pasid = pasid + }; - vtd_as = g_hash_table_find(s->vtd_address_spaces, vtd_find_as_by_sid, &sid); - s->vtd_as_cache[bus_num] = vtd_as; + return g_hash_table_find(s->vtd_address_spaces, + vtd_find_as_by_sid_and_pasid, &key); +} - return vtd_as; +static VTDAddressSpace *vtd_get_as_by_sid(IntelIOMMUState *s, uint16_t sid) +{ + return vtd_get_as_by_sid_and_pasid(s, sid, PCI_NO_PASID); } static void vtd_pt_enable_fast_path(IntelIOMMUState *s, uint16_t source_id)