RE: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
>-Original Message- >From: Eric Auger >Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >pci_device_set/unset_iommu_device() > > > >On 1/23/24 10:25, Duan, Zhenzhong wrote: >> >>> -Original Message- >>> From: Cédric Le Goater >>> Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >>> pci_device_set/unset_iommu_device() >>> >>> On 1/23/24 07:37, Duan, Zhenzhong wrote: >>>> >>>>> -Original Message- >>>>> From: Cédric Le Goater >>>>> Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >>>>> pci_device_set/unset_iommu_device() >>>>> >>>>> On 1/15/24 11:13, Zhenzhong Duan wrote: >>>>>> From: Yi Liu >>>>>> >>>>>> This adds pci_device_set/unset_iommu_device() to set/unset >>>>>> IOMMUFDDevice for a given PCIe device. Caller of set >>>>>> should fail if set operation fails. >>>>>> >>>>>> Extract out pci_device_get_iommu_bus_devfn() to facilitate >>>>>> implementation of pci_device_set/unset_iommu_device(). >>>>>> >>>>>> Signed-off-by: Yi Liu >>>>>> Signed-off-by: Yi Sun >>>>>> Signed-off-by: Nicolin Chen >>>>>> Signed-off-by: Zhenzhong Duan >>>>>> --- >>>>>>include/hw/pci/pci.h | 39 >>> ++- >>>>>>hw/pci/pci.c | 49 >>>>> +++- >>>>>>2 files changed, 86 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >>>>>> index fa6313aabc..a810c0ec74 100644 >>>>>> --- a/include/hw/pci/pci.h >>>>>> +++ b/include/hw/pci/pci.h >>>>>> @@ -7,6 +7,8 @@ >>>>>>/* PCI includes legacy ISA access. */ >>>>>>#include "hw/isa/isa.h" >>>>>> >>>>>> +#include "sysemu/iommufd_device.h" >>>>>> + >>>>>>extern bool pci_available; >>>>>> >>>>>>/* PCI bus */ >>>>>> @@ -384,10 +386,45 @@ typedef struct PCIIOMMUOps { >>>>>> * >>>>>> * @devfn: device and function number >>>>>> */ >>>>>> - AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, >int >>>>> devfn); >>>>>> +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, >>> int >>>>> devfn); >>>>>> +/** >>>>>> + * @set_iommu_device: set iommufd device for a PCI device to >>>>> vIOMMU >>>>>> + * >>>>>> + * Optional callback, if not implemented in vIOMMU, then >vIOMMU >>>>> can't >>>>>> + * utilize iommufd specific features. >>>>>> + * >>>>>> + * Return true if iommufd device is accepted, or else return false >with >>>>>> + * errp set. >>>>>> + * >>>>>> + * @bus: the #PCIBus of the PCI device. >>>>>> + * >>>>>> + * @opaque: the data passed to pci_setup_iommu(). >>>>>> + * >>>>>> + * @devfn: device and function number of the PCI device. >>>>>> + * >>>>>> + * @idev: the data structure representing iommufd device. >>>>>> + * >>>>>> + */ >>>>>> +int (*set_iommu_device)(PCIBus *bus, void *opaque, int32_t >devfn, >>>>>> +IOMMUFDDevice *idev, Error **errp); >>>>>> +/** >>>>>> + * @unset_iommu_device: unset iommufd device for a PCI device >>> from >>>>> vIOMMU >>>>>> + * >>>>>> + * Optional callback. >>>>>> + * >>>>>> + * @bus: the #PCIBus of the PCI device. >>>>>> + * >>>>>> + * @opaque: the data passed to pci_setup_iommu(). >>>>>> + * >>>>>> + * @devfn: device and function number of the PCI device. >>>>>> + */ >>>>>> +void (*unset_iommu_device)(PCIBus *bus, void *opaq
Re: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
On 1/23/24 10:25, Duan, Zhenzhong wrote: > >> -Original Message- >> From: Cédric Le Goater >> Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >> pci_device_set/unset_iommu_device() >> >> On 1/23/24 07:37, Duan, Zhenzhong wrote: >>> >>>> -Original Message- >>>> From: Cédric Le Goater >>>> Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >>>> pci_device_set/unset_iommu_device() >>>> >>>> On 1/15/24 11:13, Zhenzhong Duan wrote: >>>>> From: Yi Liu >>>>> >>>>> This adds pci_device_set/unset_iommu_device() to set/unset >>>>> IOMMUFDDevice for a given PCIe device. Caller of set >>>>> should fail if set operation fails. >>>>> >>>>> Extract out pci_device_get_iommu_bus_devfn() to facilitate >>>>> implementation of pci_device_set/unset_iommu_device(). >>>>> >>>>> Signed-off-by: Yi Liu >>>>> Signed-off-by: Yi Sun >>>>> Signed-off-by: Nicolin Chen >>>>> Signed-off-by: Zhenzhong Duan >>>>> --- >>>>>include/hw/pci/pci.h | 39 >> ++- >>>>>hw/pci/pci.c | 49 >>>> +++- >>>>>2 files changed, 86 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >>>>> index fa6313aabc..a810c0ec74 100644 >>>>> --- a/include/hw/pci/pci.h >>>>> +++ b/include/hw/pci/pci.h >>>>> @@ -7,6 +7,8 @@ >>>>>/* PCI includes legacy ISA access. */ >>>>>#include "hw/isa/isa.h" >>>>> >>>>> +#include "sysemu/iommufd_device.h" >>>>> + >>>>>extern bool pci_available; >>>>> >>>>>/* PCI bus */ >>>>> @@ -384,10 +386,45 @@ typedef struct PCIIOMMUOps { >>>>> * >>>>> * @devfn: device and function number >>>>> */ >>>>> - AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int >>>> devfn); >>>>> +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, >> int >>>> devfn); >>>>> +/** >>>>> + * @set_iommu_device: set iommufd device for a PCI device to >>>> vIOMMU >>>>> + * >>>>> + * Optional callback, if not implemented in vIOMMU, then vIOMMU >>>> can't >>>>> + * utilize iommufd specific features. >>>>> + * >>>>> + * Return true if iommufd device is accepted, or else return false >>>>> with >>>>> + * errp set. >>>>> + * >>>>> + * @bus: the #PCIBus of the PCI device. >>>>> + * >>>>> + * @opaque: the data passed to pci_setup_iommu(). >>>>> + * >>>>> + * @devfn: device and function number of the PCI device. >>>>> + * >>>>> + * @idev: the data structure representing iommufd device. >>>>> + * >>>>> + */ >>>>> +int (*set_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn, >>>>> +IOMMUFDDevice *idev, Error **errp); >>>>> +/** >>>>> + * @unset_iommu_device: unset iommufd device for a PCI device >> from >>>> vIOMMU >>>>> + * >>>>> + * Optional callback. >>>>> + * >>>>> + * @bus: the #PCIBus of the PCI device. >>>>> + * >>>>> + * @opaque: the data passed to pci_setup_iommu(). >>>>> + * >>>>> + * @devfn: device and function number of the PCI device. >>>>> + */ >>>>> +void (*unset_iommu_device)(PCIBus *bus, void *opaque, int32_t >>>> devfn); >>>>>} PCIIOMMUOps; >>>>> >>>>>AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); >>>>> +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice >>>> *idev, >>>>> +Error **errp); >>>>> +void pci_device_unset_iommu_device(PCIDevice *dev); >>>>> >>>>>/** >>>>>
RE: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
>-Original Message- >From: Cédric Le Goater >Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >pci_device_set/unset_iommu_device() > >On 1/23/24 07:37, Duan, Zhenzhong wrote: >> >> >>> -Original Message- >>> From: Cédric Le Goater >>> Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >>> pci_device_set/unset_iommu_device() >>> >>> On 1/15/24 11:13, Zhenzhong Duan wrote: >>>> From: Yi Liu >>>> >>>> This adds pci_device_set/unset_iommu_device() to set/unset >>>> IOMMUFDDevice for a given PCIe device. Caller of set >>>> should fail if set operation fails. >>>> >>>> Extract out pci_device_get_iommu_bus_devfn() to facilitate >>>> implementation of pci_device_set/unset_iommu_device(). >>>> >>>> Signed-off-by: Yi Liu >>>> Signed-off-by: Yi Sun >>>> Signed-off-by: Nicolin Chen >>>> Signed-off-by: Zhenzhong Duan >>>> --- >>>>include/hw/pci/pci.h | 39 >++- >>>>hw/pci/pci.c | 49 >>> +++- >>>>2 files changed, 86 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >>>> index fa6313aabc..a810c0ec74 100644 >>>> --- a/include/hw/pci/pci.h >>>> +++ b/include/hw/pci/pci.h >>>> @@ -7,6 +7,8 @@ >>>>/* PCI includes legacy ISA access. */ >>>>#include "hw/isa/isa.h" >>>> >>>> +#include "sysemu/iommufd_device.h" >>>> + >>>>extern bool pci_available; >>>> >>>>/* PCI bus */ >>>> @@ -384,10 +386,45 @@ typedef struct PCIIOMMUOps { >>>> * >>>> * @devfn: device and function number >>>> */ >>>> - AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int >>> devfn); >>>> +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, >int >>> devfn); >>>> +/** >>>> + * @set_iommu_device: set iommufd device for a PCI device to >>> vIOMMU >>>> + * >>>> + * Optional callback, if not implemented in vIOMMU, then vIOMMU >>> can't >>>> + * utilize iommufd specific features. >>>> + * >>>> + * Return true if iommufd device is accepted, or else return false >>>> with >>>> + * errp set. >>>> + * >>>> + * @bus: the #PCIBus of the PCI device. >>>> + * >>>> + * @opaque: the data passed to pci_setup_iommu(). >>>> + * >>>> + * @devfn: device and function number of the PCI device. >>>> + * >>>> + * @idev: the data structure representing iommufd device. >>>> + * >>>> + */ >>>> +int (*set_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn, >>>> +IOMMUFDDevice *idev, Error **errp); >>>> +/** >>>> + * @unset_iommu_device: unset iommufd device for a PCI device >from >>> vIOMMU >>>> + * >>>> + * Optional callback. >>>> + * >>>> + * @bus: the #PCIBus of the PCI device. >>>> + * >>>> + * @opaque: the data passed to pci_setup_iommu(). >>>> + * >>>> + * @devfn: device and function number of the PCI device. >>>> + */ >>>> +void (*unset_iommu_device)(PCIBus *bus, void *opaque, int32_t >>> devfn); >>>>} PCIIOMMUOps; >>>> >>>>AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); >>>> +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice >>> *idev, >>>> +Error **errp); >>>> +void pci_device_unset_iommu_device(PCIDevice *dev); >>>> >>>>/** >>>> * pci_setup_iommu: Initialize specific IOMMU handlers for a PCIBus >>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >>>> index 76080af580..3848662f95 100644 >>>> --- a/hw/pci/pci.c >>>> +++ b/hw/pci/pci.c >>>> @@ -2672,7 +2672,10 @@ static void >>> pci_device_class_base_init(ObjectClass *klass, void *data) >>>>} >>>>} >>>> >>>> -AddressSpace *pci_device
Re: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
On 1/23/24 07:37, Duan, Zhenzhong wrote: -Original Message- From: Cédric Le Goater Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device() On 1/15/24 11:13, Zhenzhong Duan wrote: From: Yi Liu This adds pci_device_set/unset_iommu_device() to set/unset IOMMUFDDevice for a given PCIe device. Caller of set should fail if set operation fails. Extract out pci_device_get_iommu_bus_devfn() to facilitate implementation of pci_device_set/unset_iommu_device(). Signed-off-by: Yi Liu Signed-off-by: Yi Sun Signed-off-by: Nicolin Chen Signed-off-by: Zhenzhong Duan --- include/hw/pci/pci.h | 39 ++- hw/pci/pci.c | 49 +++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index fa6313aabc..a810c0ec74 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -7,6 +7,8 @@ /* PCI includes legacy ISA access. */ #include "hw/isa/isa.h" +#include "sysemu/iommufd_device.h" + extern bool pci_available; /* PCI bus */ @@ -384,10 +386,45 @@ typedef struct PCIIOMMUOps { * * @devfn: device and function number */ - AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int devfn); +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int devfn); +/** + * @set_iommu_device: set iommufd device for a PCI device to vIOMMU + * + * Optional callback, if not implemented in vIOMMU, then vIOMMU can't + * utilize iommufd specific features. + * + * Return true if iommufd device is accepted, or else return false with + * errp set. + * + * @bus: the #PCIBus of the PCI device. + * + * @opaque: the data passed to pci_setup_iommu(). + * + * @devfn: device and function number of the PCI device. + * + * @idev: the data structure representing iommufd device. + * + */ +int (*set_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn, +IOMMUFDDevice *idev, Error **errp); +/** + * @unset_iommu_device: unset iommufd device for a PCI device from vIOMMU + * + * Optional callback. + * + * @bus: the #PCIBus of the PCI device. + * + * @opaque: the data passed to pci_setup_iommu(). + * + * @devfn: device and function number of the PCI device. + */ +void (*unset_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn); } PCIIOMMUOps; AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice *idev, +Error **errp); +void pci_device_unset_iommu_device(PCIDevice *dev); /** * pci_setup_iommu: Initialize specific IOMMU handlers for a PCIBus diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 76080af580..3848662f95 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2672,7 +2672,10 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) } } -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, + PCIBus **aliased_pbus, + PCIBus **piommu_bus, + uint8_t *aliased_pdevfn) { PCIBus *bus = pci_get_bus(dev); PCIBus *iommu_bus = bus; @@ -2717,6 +2720,18 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) iommu_bus = parent_bus; } +*aliased_pbus = bus; +*piommu_bus = iommu_bus; +*aliased_pdevfn = devfn; +} + +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) +{ +PCIBus *bus; +PCIBus *iommu_bus; +uint8_t devfn; + +pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); if (!pci_bus_bypass_iommu(bus) && iommu_bus->iommu_ops) { return iommu_bus->iommu_ops->get_address_space(bus, iommu_bus->iommu_opaque, devfn); @@ -2724,6 +2739,38 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) return &address_space_memory; } +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice *idev, +Error **errp) +{ +PCIBus *bus; +PCIBus *iommu_bus; +uint8_t devfn; + +pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); +if (!pci_bus_bypass_iommu(bus) && iommu_bus && Why do we test iommu_bus in pci_device_un/set_iommu_device routines and not in pci_device_iommu_address_space() ? iommu_bus check in pci_device_iommu_address_space() is dropped in below commit, I didn't find related discussion in mail history, maybe by accident? I can add it back if it's not intentional. Can iommu_bus be NULL or should we add an assert ? C. ba7d12eb8c hw/pci: modify pci_setup_iommu() to set PCIIOMMUOps Thanks Zhenzhong
RE: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
>-Original Message- >From: Cédric Le Goater >Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >pci_device_set/unset_iommu_device() > >On 1/15/24 11:13, Zhenzhong Duan wrote: >> From: Yi Liu >> >> This adds pci_device_set/unset_iommu_device() to set/unset >> IOMMUFDDevice for a given PCIe device. Caller of set >> should fail if set operation fails. >> >> Extract out pci_device_get_iommu_bus_devfn() to facilitate >> implementation of pci_device_set/unset_iommu_device(). >> >> Signed-off-by: Yi Liu >> Signed-off-by: Yi Sun >> Signed-off-by: Nicolin Chen >> Signed-off-by: Zhenzhong Duan >> --- >> include/hw/pci/pci.h | 39 ++- >> hw/pci/pci.c | 49 >+++- >> 2 files changed, 86 insertions(+), 2 deletions(-) >> >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index fa6313aabc..a810c0ec74 100644 >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -7,6 +7,8 @@ >> /* PCI includes legacy ISA access. */ >> #include "hw/isa/isa.h" >> >> +#include "sysemu/iommufd_device.h" >> + >> extern bool pci_available; >> >> /* PCI bus */ >> @@ -384,10 +386,45 @@ typedef struct PCIIOMMUOps { >>* >>* @devfn: device and function number >>*/ >> - AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int >devfn); >> +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int >devfn); >> +/** >> + * @set_iommu_device: set iommufd device for a PCI device to >vIOMMU >> + * >> + * Optional callback, if not implemented in vIOMMU, then vIOMMU >can't >> + * utilize iommufd specific features. >> + * >> + * Return true if iommufd device is accepted, or else return false with >> + * errp set. >> + * >> + * @bus: the #PCIBus of the PCI device. >> + * >> + * @opaque: the data passed to pci_setup_iommu(). >> + * >> + * @devfn: device and function number of the PCI device. >> + * >> + * @idev: the data structure representing iommufd device. >> + * >> + */ >> +int (*set_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn, >> +IOMMUFDDevice *idev, Error **errp); >> +/** >> + * @unset_iommu_device: unset iommufd device for a PCI device from >vIOMMU >> + * >> + * Optional callback. >> + * >> + * @bus: the #PCIBus of the PCI device. >> + * >> + * @opaque: the data passed to pci_setup_iommu(). >> + * >> + * @devfn: device and function number of the PCI device. >> + */ >> +void (*unset_iommu_device)(PCIBus *bus, void *opaque, int32_t >devfn); >> } PCIIOMMUOps; >> >> AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); >> +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice >*idev, >> +Error **errp); >> +void pci_device_unset_iommu_device(PCIDevice *dev); >> >> /** >>* pci_setup_iommu: Initialize specific IOMMU handlers for a PCIBus >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 76080af580..3848662f95 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -2672,7 +2672,10 @@ static void >pci_device_class_base_init(ObjectClass *klass, void *data) >> } >> } >> >> -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) >> +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, >> + PCIBus **aliased_pbus, >> + PCIBus **piommu_bus, >> + uint8_t *aliased_pdevfn) >> { >> PCIBus *bus = pci_get_bus(dev); >> PCIBus *iommu_bus = bus; >> @@ -2717,6 +2720,18 @@ AddressSpace >*pci_device_iommu_address_space(PCIDevice *dev) >> >> iommu_bus = parent_bus; >> } >> +*aliased_pbus = bus; >> +*piommu_bus = iommu_bus; >> +*aliased_pdevfn = devfn; >> +} >> + >> +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) >> +{ >> +PCIBus *bus; >> +PCIBus *iommu_bus; >> +uint8_t devfn; >> + >> +pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); >> if (!pci_bus_bypass_iommu(bus) && iommu_bu
Re: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
On 1/15/24 11:13, Zhenzhong Duan wrote: From: Yi Liu This adds pci_device_set/unset_iommu_device() to set/unset IOMMUFDDevice for a given PCIe device. Caller of set should fail if set operation fails. Extract out pci_device_get_iommu_bus_devfn() to facilitate implementation of pci_device_set/unset_iommu_device(). Signed-off-by: Yi Liu Signed-off-by: Yi Sun Signed-off-by: Nicolin Chen Signed-off-by: Zhenzhong Duan --- include/hw/pci/pci.h | 39 ++- hw/pci/pci.c | 49 +++- 2 files changed, 86 insertions(+), 2 deletions(-) diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h index fa6313aabc..a810c0ec74 100644 --- a/include/hw/pci/pci.h +++ b/include/hw/pci/pci.h @@ -7,6 +7,8 @@ /* PCI includes legacy ISA access. */ #include "hw/isa/isa.h" +#include "sysemu/iommufd_device.h" + extern bool pci_available; /* PCI bus */ @@ -384,10 +386,45 @@ typedef struct PCIIOMMUOps { * * @devfn: device and function number */ - AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int devfn); +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int devfn); +/** + * @set_iommu_device: set iommufd device for a PCI device to vIOMMU + * + * Optional callback, if not implemented in vIOMMU, then vIOMMU can't + * utilize iommufd specific features. + * + * Return true if iommufd device is accepted, or else return false with + * errp set. + * + * @bus: the #PCIBus of the PCI device. + * + * @opaque: the data passed to pci_setup_iommu(). + * + * @devfn: device and function number of the PCI device. + * + * @idev: the data structure representing iommufd device. + * + */ +int (*set_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn, +IOMMUFDDevice *idev, Error **errp); +/** + * @unset_iommu_device: unset iommufd device for a PCI device from vIOMMU + * + * Optional callback. + * + * @bus: the #PCIBus of the PCI device. + * + * @opaque: the data passed to pci_setup_iommu(). + * + * @devfn: device and function number of the PCI device. + */ +void (*unset_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn); } PCIIOMMUOps; AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice *idev, +Error **errp); +void pci_device_unset_iommu_device(PCIDevice *dev); /** * pci_setup_iommu: Initialize specific IOMMU handlers for a PCIBus diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 76080af580..3848662f95 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -2672,7 +2672,10 @@ static void pci_device_class_base_init(ObjectClass *klass, void *data) } } -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, + PCIBus **aliased_pbus, + PCIBus **piommu_bus, + uint8_t *aliased_pdevfn) { PCIBus *bus = pci_get_bus(dev); PCIBus *iommu_bus = bus; @@ -2717,6 +2720,18 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) iommu_bus = parent_bus; } +*aliased_pbus = bus; +*piommu_bus = iommu_bus; +*aliased_pdevfn = devfn; +} + +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) +{ +PCIBus *bus; +PCIBus *iommu_bus; +uint8_t devfn; + +pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); if (!pci_bus_bypass_iommu(bus) && iommu_bus->iommu_ops) { return iommu_bus->iommu_ops->get_address_space(bus, iommu_bus->iommu_opaque, devfn); @@ -2724,6 +2739,38 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) return &address_space_memory; } +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice *idev, +Error **errp) +{ +PCIBus *bus; +PCIBus *iommu_bus; +uint8_t devfn; + +pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); +if (!pci_bus_bypass_iommu(bus) && iommu_bus && Why do we test iommu_bus in pci_device_un/set_iommu_device routines and not in pci_device_iommu_address_space() ? Thanks, C. +iommu_bus->iommu_ops && iommu_bus->iommu_ops->set_iommu_device) { +return iommu_bus->iommu_ops->set_iommu_device(pci_get_bus(dev), + iommu_bus->iommu_opaque, + dev->devfn, idev, errp); +} +return 0; +} + +void pci_device_unset_iommu_device(PCIDevice *dev) +{ +PCIBus *bus; +PCIBus *iommu_bus; +uint8_t devfn; + +pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); +if (!p
RE: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
>-Original Message- >From: Eric Auger >Subject: Re: [PATCH rfcv1 2/6] hw/pci: introduce >pci_device_set/unset_iommu_device() > >Hi Zhenzhong, > >On 1/15/24 11:13, Zhenzhong Duan wrote: >> From: Yi Liu >> >> This adds pci_device_set/unset_iommu_device() to set/unset >> IOMMUFDDevice for a given PCIe device. Caller of set >> should fail if set operation fails. >> >> Extract out pci_device_get_iommu_bus_devfn() to facilitate >> implementation of pci_device_set/unset_iommu_device(). >> >> Signed-off-by: Yi Liu >> Signed-off-by: Yi Sun >> Signed-off-by: Nicolin Chen >> Signed-off-by: Zhenzhong Duan >> --- >> include/hw/pci/pci.h | 39 ++- >> hw/pci/pci.c | 49 >+++- >> 2 files changed, 86 insertions(+), 2 deletions(-) >> >> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h >> index fa6313aabc..a810c0ec74 100644 >> --- a/include/hw/pci/pci.h >> +++ b/include/hw/pci/pci.h >> @@ -7,6 +7,8 @@ >> /* PCI includes legacy ISA access. */ >> #include "hw/isa/isa.h" >> >> +#include "sysemu/iommufd_device.h" >> + >> extern bool pci_available; >> >> /* PCI bus */ >> @@ -384,10 +386,45 @@ typedef struct PCIIOMMUOps { >> * >> * @devfn: device and function number >> */ >> - AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int >devfn); >> +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int >devfn); >> +/** >> + * @set_iommu_device: set iommufd device for a PCI device to >vIOMMU >> + * >> + * Optional callback, if not implemented in vIOMMU, then vIOMMU >can't >> + * utilize iommufd specific features. >> + * >> + * Return true if iommufd device is accepted, or else return false with >> + * errp set. >> + * >> + * @bus: the #PCIBus of the PCI device. >> + * >> + * @opaque: the data passed to pci_setup_iommu(). >> + * >> + * @devfn: device and function number of the PCI device. >> + * >> + * @idev: the data structure representing iommufd device. >> + * >> + */ >> +int (*set_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn, >> +IOMMUFDDevice *idev, Error **errp); >> +/** >> + * @unset_iommu_device: unset iommufd device for a PCI device from >vIOMMU >> + * >> + * Optional callback. >> + * >> + * @bus: the #PCIBus of the PCI device. >> + * >> + * @opaque: the data passed to pci_setup_iommu(). >> + * >> + * @devfn: device and function number of the PCI device. >> + */ >> +void (*unset_iommu_device)(PCIBus *bus, void *opaque, int32_t >devfn); >> } PCIIOMMUOps; >> >> AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); >> +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice >*idev, >> +Error **errp); >> +void pci_device_unset_iommu_device(PCIDevice *dev); >> >> /** >> * pci_setup_iommu: Initialize specific IOMMU handlers for a PCIBus >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 76080af580..3848662f95 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -2672,7 +2672,10 @@ static void >pci_device_class_base_init(ObjectClass *klass, void *data) >> } >> } >> >> -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) >> +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, >> + PCIBus **aliased_pbus, >> + PCIBus **piommu_bus, >> + uint8_t *aliased_pdevfn) >nit: I would drop the p in aliased_pbus andaliased_pdevfn. Maybe you >should allow the caller to pass NUL for aliased_pbus and aliased_pdevfn >as it is the case for pci_device_set_iommu_device() I may resue that >helper in [RFC 2/7] hw/pci: Introduce pci_device_iommu_bus Good suggestion, will do. Thanks Zhenzhong >> { >> PCIBus *bus = pci_get_bus(dev); >> PCIBus *iommu_bus = bus; >> @@ -2717,6 +2720,18 @@ AddressSpace >*pci_device_iommu_address_space(PCIDevice *dev) >> >> iommu_bus = parent_bus; >> } >> +*aliased_pbus = bus; >> +*piommu_bus = iommu_bus; >> +*aliased_pdevfn = devfn; >> +} >> + >> +AddressSpace *pci_device_io
Re: [PATCH rfcv1 2/6] hw/pci: introduce pci_device_set/unset_iommu_device()
Hi Zhenzhong, On 1/15/24 11:13, Zhenzhong Duan wrote: > From: Yi Liu > > This adds pci_device_set/unset_iommu_device() to set/unset > IOMMUFDDevice for a given PCIe device. Caller of set > should fail if set operation fails. > > Extract out pci_device_get_iommu_bus_devfn() to facilitate > implementation of pci_device_set/unset_iommu_device(). > > Signed-off-by: Yi Liu > Signed-off-by: Yi Sun > Signed-off-by: Nicolin Chen > Signed-off-by: Zhenzhong Duan > --- > include/hw/pci/pci.h | 39 ++- > hw/pci/pci.c | 49 +++- > 2 files changed, 86 insertions(+), 2 deletions(-) > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index fa6313aabc..a810c0ec74 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -7,6 +7,8 @@ > /* PCI includes legacy ISA access. */ > #include "hw/isa/isa.h" > > +#include "sysemu/iommufd_device.h" > + > extern bool pci_available; > > /* PCI bus */ > @@ -384,10 +386,45 @@ typedef struct PCIIOMMUOps { > * > * @devfn: device and function number > */ > - AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int devfn); > +AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque, int > devfn); > +/** > + * @set_iommu_device: set iommufd device for a PCI device to vIOMMU > + * > + * Optional callback, if not implemented in vIOMMU, then vIOMMU can't > + * utilize iommufd specific features. > + * > + * Return true if iommufd device is accepted, or else return false with > + * errp set. > + * > + * @bus: the #PCIBus of the PCI device. > + * > + * @opaque: the data passed to pci_setup_iommu(). > + * > + * @devfn: device and function number of the PCI device. > + * > + * @idev: the data structure representing iommufd device. > + * > + */ > +int (*set_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn, > +IOMMUFDDevice *idev, Error **errp); > +/** > + * @unset_iommu_device: unset iommufd device for a PCI device from vIOMMU > + * > + * Optional callback. > + * > + * @bus: the #PCIBus of the PCI device. > + * > + * @opaque: the data passed to pci_setup_iommu(). > + * > + * @devfn: device and function number of the PCI device. > + */ > +void (*unset_iommu_device)(PCIBus *bus, void *opaque, int32_t devfn); > } PCIIOMMUOps; > > AddressSpace *pci_device_iommu_address_space(PCIDevice *dev); > +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice *idev, > +Error **errp); > +void pci_device_unset_iommu_device(PCIDevice *dev); > > /** > * pci_setup_iommu: Initialize specific IOMMU handlers for a PCIBus > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 76080af580..3848662f95 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -2672,7 +2672,10 @@ static void pci_device_class_base_init(ObjectClass > *klass, void *data) > } > } > > -AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +static void pci_device_get_iommu_bus_devfn(PCIDevice *dev, > + PCIBus **aliased_pbus, > + PCIBus **piommu_bus, > + uint8_t *aliased_pdevfn) nit: I would drop the p in aliased_pbus andaliased_pdevfn. Maybe you should allow the caller to pass NUL for aliased_pbus and aliased_pdevfn as it is the case for pci_device_set_iommu_device() I may resue that helper in [RFC 2/7] hw/pci: Introduce pci_device_iommu_bus > { > PCIBus *bus = pci_get_bus(dev); > PCIBus *iommu_bus = bus; > @@ -2717,6 +2720,18 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice > *dev) > > iommu_bus = parent_bus; > } > +*aliased_pbus = bus; > +*piommu_bus = iommu_bus; > +*aliased_pdevfn = devfn; > +} > + > +AddressSpace *pci_device_iommu_address_space(PCIDevice *dev) > +{ > +PCIBus *bus; > +PCIBus *iommu_bus; > +uint8_t devfn; > + > +pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); > if (!pci_bus_bypass_iommu(bus) && iommu_bus->iommu_ops) { > return iommu_bus->iommu_ops->get_address_space(bus, > iommu_bus->iommu_opaque, devfn); > @@ -2724,6 +2739,38 @@ AddressSpace *pci_device_iommu_address_space(PCIDevice > *dev) > return &address_space_memory; > } > > +int pci_device_set_iommu_device(PCIDevice *dev, IOMMUFDDevice *idev, > +Error **errp) > +{ > +PCIBus *bus; > +PCIBus *iommu_bus; > +uint8_t devfn; > + > +pci_device_get_iommu_bus_devfn(dev, &bus, &iommu_bus, &devfn); > +if (!pci_bus_bypass_iommu(bus) && iommu_bus && > +iommu_bus->iommu_ops && iommu_bus->iommu_ops->set_iommu_device) { > +return iommu_bus->iommu_ops->set_iommu_device(pci_get_bus(dev), > +