RE: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
Hi Michael,
>-Original Message-
>From: Michael S. Tsirkin
>Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>sync host IOMMU cap/ecap
>
>On Mon, Mar 18, 2024 at 02:20:50PM +0100, Eric Auger wrote:
>> Hi Michael,
>>
>> On 3/13/24 12:17, Michael S. Tsirkin wrote:
>> > On Wed, Mar 13, 2024 at 07:54:11AM +, Duan, Zhenzhong wrote:
>> >>
>> >>> -Original Message-
>> >>> From: Michael S. Tsirkin
>> >>> Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check
>and
>> >>> sync host IOMMU cap/ecap
>> >>>
>> >>> On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
>> Hi Michael,
>>
>> > -Original Message-
>> > From: Michael S. Tsirkin
>> > Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to
>check and
>> > sync host IOMMU cap/ecap
>> >
>> > On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan
>wrote:
>> >> From: Yi Liu
>> >>
>> >> Add a framework to check and synchronize host IOMMU cap/ecap
>with
>> >> vIOMMU cap/ecap.
>> >>
>> >> The sequence will be:
>> >>
>> >> vtd_cap_init() initializes iommu->cap/ecap.
>> >> vtd_check_hdev() update iommu->cap/ecap based on host
>cap/ecap.
>> >> iommu->cap_frozen set when machine create done, iommu-
>>cap/ecap
>> > become readonly.
>> >> Implementation details for different backends will be in following
>> >>> patches.
>> >> Signed-off-by: Yi Liu
>> >> Signed-off-by: Yi Sun
>> >> Signed-off-by: Zhenzhong Duan
>> >> ---
>> >> include/hw/i386/intel_iommu.h | 1 +
>> >> hw/i386/intel_iommu.c | 50
>> > ++-
>> >> 2 files changed, 50 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/include/hw/i386/intel_iommu.h
>> > b/include/hw/i386/intel_iommu.h
>> >> index bbc7b96add..c71a133820 100644
>> >> --- a/include/hw/i386/intel_iommu.h
>> >> +++ b/include/hw/i386/intel_iommu.h
>> >> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
>> >>
>> >> uint64_t cap; /* The value of capability reg */
>> >> uint64_t ecap; /* The value of extended
>> >> capability reg
>*/
>> >> +bool cap_frozen;/* cap/ecap become read-only
>> >> after
>> >>> frozen */
>> >> uint32_t context_cache_gen; /* Should be in [1,MAX] */
>> >> GHashTable *iotlb; /* IOTLB */
>> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> >> index ffa1ad6429..a9f9dfd6a7 100644
>> >> --- a/hw/i386/intel_iommu.c
>> >> +++ b/hw/i386/intel_iommu.c
>> >> @@ -35,6 +35,8 @@
>> >> #include "sysemu/kvm.h"
>> >> #include "sysemu/dma.h"
>> >> #include "sysemu/sysemu.h"
>> >> +#include "hw/vfio/vfio-common.h"
>> >> +#include "sysemu/iommufd.h"
>> >> #include "hw/i386/apic_internal.h"
>> >> #include "kvm/kvm_i386.h"
>> >> #include "migration/vmstate.h"
>> >> @@ -3819,6 +3821,38 @@ VTDAddressSpace
>> > *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>> >> return vtd_dev_as;
>> >> }
>> >>
>> >> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>> >> + IOMMULegacyDevice *ldev,
>> >> + Error **errp)
>> >> +{
>> >> +return 0;
>> >> +}
>> >> +
>> >> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>> >> + IOMMUFDDevice *idev,
>> >> + Error **errp)
>> >> +{
>> >> +return 0;
>> >> +}
>> >> +
>> >> +static int vtd_check_hdev(IntelIOMMUState *s,
>> >>> VTDHostIOMMUDevice
>> > *vtd_hdev,
>> >> + Error **errp)
>> >> +{
>> >> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
>> >> +IOMMUFDDevice *idev;
>> >> +
>> >> +if (base_dev->type == HID_LEGACY) {
>> >> +IOMMULegacyDevice *ldev = container_of(base_dev,
>> >> + IOMMULegacyDevice,
>> >> base);
>> >> +
>> >> +return vtd_check_legacy_hdev(s, ldev, errp);
>> >> +}
>> >> +
>> >> +idev = container_of(base_dev, IOMMUFDDevice, base);
>> >> +
>> >> +return vtd_check_iommufd_hdev(s, idev, errp);
>> >> +}
>> >> +
>> >> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque,
>int
>> > devfn,
>> >> HostIOMMUDevice *base_dev, Error
>> >> **errp)
>> >> {
>> >> @@ -3829,6 +3863,7 @@ static int
>> >>> vtd_dev_set_iommu_device(PCIBus
>> > *bus, void *opaque, int devfn,
>> >> .devfn = devfn,
>> >> };
>> >> struct vtd_as_key *new_key;
>> >> +int ret;
>> >>
>> >> assert(base_dev);
>> >>
>> >> @@ -3848,6 +3883,13 @@ static int
Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
On Mon, Mar 18, 2024 at 02:20:50PM +0100, Eric Auger wrote:
> Hi Michael,
>
> On 3/13/24 12:17, Michael S. Tsirkin wrote:
> > On Wed, Mar 13, 2024 at 07:54:11AM +, Duan, Zhenzhong wrote:
> >>
> >>> -Original Message-
> >>> From: Michael S. Tsirkin
> >>> Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
> >>> sync host IOMMU cap/ecap
> >>>
> >>> On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
> Hi Michael,
>
> > -Original Message-
> > From: Michael S. Tsirkin
> > Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
> > sync host IOMMU cap/ecap
> >
> > On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
> >> From: Yi Liu
> >>
> >> Add a framework to check and synchronize host IOMMU cap/ecap with
> >> vIOMMU cap/ecap.
> >>
> >> The sequence will be:
> >>
> >> vtd_cap_init() initializes iommu->cap/ecap.
> >> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
> >> iommu->cap_frozen set when machine create done, iommu->cap/ecap
> > become readonly.
> >> Implementation details for different backends will be in following
> >>> patches.
> >> Signed-off-by: Yi Liu
> >> Signed-off-by: Yi Sun
> >> Signed-off-by: Zhenzhong Duan
> >> ---
> >> include/hw/i386/intel_iommu.h | 1 +
> >> hw/i386/intel_iommu.c | 50
> > ++-
> >> 2 files changed, 50 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/hw/i386/intel_iommu.h
> > b/include/hw/i386/intel_iommu.h
> >> index bbc7b96add..c71a133820 100644
> >> --- a/include/hw/i386/intel_iommu.h
> >> +++ b/include/hw/i386/intel_iommu.h
> >> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
> >>
> >> uint64_t cap; /* The value of capability reg */
> >> uint64_t ecap; /* The value of extended
> >> capability reg */
> >> +bool cap_frozen;/* cap/ecap become read-only after
> >>> frozen */
> >> uint32_t context_cache_gen; /* Should be in [1,MAX] */
> >> GHashTable *iotlb; /* IOTLB */
> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> >> index ffa1ad6429..a9f9dfd6a7 100644
> >> --- a/hw/i386/intel_iommu.c
> >> +++ b/hw/i386/intel_iommu.c
> >> @@ -35,6 +35,8 @@
> >> #include "sysemu/kvm.h"
> >> #include "sysemu/dma.h"
> >> #include "sysemu/sysemu.h"
> >> +#include "hw/vfio/vfio-common.h"
> >> +#include "sysemu/iommufd.h"
> >> #include "hw/i386/apic_internal.h"
> >> #include "kvm/kvm_i386.h"
> >> #include "migration/vmstate.h"
> >> @@ -3819,6 +3821,38 @@ VTDAddressSpace
> > *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
> >> return vtd_dev_as;
> >> }
> >>
> >> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
> >> + IOMMULegacyDevice *ldev,
> >> + Error **errp)
> >> +{
> >> +return 0;
> >> +}
> >> +
> >> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
> >> + IOMMUFDDevice *idev,
> >> + Error **errp)
> >> +{
> >> +return 0;
> >> +}
> >> +
> >> +static int vtd_check_hdev(IntelIOMMUState *s,
> >>> VTDHostIOMMUDevice
> > *vtd_hdev,
> >> + Error **errp)
> >> +{
> >> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
> >> +IOMMUFDDevice *idev;
> >> +
> >> +if (base_dev->type == HID_LEGACY) {
> >> +IOMMULegacyDevice *ldev = container_of(base_dev,
> >> + IOMMULegacyDevice,
> >> base);
> >> +
> >> +return vtd_check_legacy_hdev(s, ldev, errp);
> >> +}
> >> +
> >> +idev = container_of(base_dev, IOMMUFDDevice, base);
> >> +
> >> +return vtd_check_iommufd_hdev(s, idev, errp);
> >> +}
> >> +
> >> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
> > devfn,
> >> HostIOMMUDevice *base_dev, Error
> >> **errp)
> >> {
> >> @@ -3829,6 +3863,7 @@ static int
> >>> vtd_dev_set_iommu_device(PCIBus
> > *bus, void *opaque, int devfn,
> >> .devfn = devfn,
> >> };
> >> struct vtd_as_key *new_key;
> >> +int ret;
> >>
> >> assert(base_dev);
> >>
> >> @@ -3848,6 +3883,13 @@ static int
> >>> vtd_dev_set_iommu_device(PCIBus
> > *bus, void *opaque, int devfn,
> >> vtd_hdev->iommu_state = s;
> >> vtd_hdev->dev = base_dev;
> >>
> >> +ret = vtd_check_hdev(s, vtd_hdev, errp);
> >> +if (ret) {
> >> +g_free(vtd_hdev);
> >> +
Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
Hi Michael,
On 3/13/24 12:17, Michael S. Tsirkin wrote:
> On Wed, Mar 13, 2024 at 07:54:11AM +, Duan, Zhenzhong wrote:
>>
>>> -Original Message-
>>> From: Michael S. Tsirkin
>>> Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>>> sync host IOMMU cap/ecap
>>>
>>> On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
Hi Michael,
> -Original Message-
> From: Michael S. Tsirkin
> Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
> sync host IOMMU cap/ecap
>
> On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
>> From: Yi Liu
>>
>> Add a framework to check and synchronize host IOMMU cap/ecap with
>> vIOMMU cap/ecap.
>>
>> The sequence will be:
>>
>> vtd_cap_init() initializes iommu->cap/ecap.
>> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
>> iommu->cap_frozen set when machine create done, iommu->cap/ecap
> become readonly.
>> Implementation details for different backends will be in following
>>> patches.
>> Signed-off-by: Yi Liu
>> Signed-off-by: Yi Sun
>> Signed-off-by: Zhenzhong Duan
>> ---
>> include/hw/i386/intel_iommu.h | 1 +
>> hw/i386/intel_iommu.c | 50
> ++-
>> 2 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/i386/intel_iommu.h
> b/include/hw/i386/intel_iommu.h
>> index bbc7b96add..c71a133820 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
>>
>> uint64_t cap; /* The value of capability reg */
>> uint64_t ecap; /* The value of extended capability
>> reg */
>> +bool cap_frozen;/* cap/ecap become read-only after
>>> frozen */
>> uint32_t context_cache_gen; /* Should be in [1,MAX] */
>> GHashTable *iotlb; /* IOTLB */
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index ffa1ad6429..a9f9dfd6a7 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -35,6 +35,8 @@
>> #include "sysemu/kvm.h"
>> #include "sysemu/dma.h"
>> #include "sysemu/sysemu.h"
>> +#include "hw/vfio/vfio-common.h"
>> +#include "sysemu/iommufd.h"
>> #include "hw/i386/apic_internal.h"
>> #include "kvm/kvm_i386.h"
>> #include "migration/vmstate.h"
>> @@ -3819,6 +3821,38 @@ VTDAddressSpace
> *vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>> return vtd_dev_as;
>> }
>>
>> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>> + IOMMULegacyDevice *ldev,
>> + Error **errp)
>> +{
>> +return 0;
>> +}
>> +
>> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>> + IOMMUFDDevice *idev,
>> + Error **errp)
>> +{
>> +return 0;
>> +}
>> +
>> +static int vtd_check_hdev(IntelIOMMUState *s,
>>> VTDHostIOMMUDevice
> *vtd_hdev,
>> + Error **errp)
>> +{
>> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
>> +IOMMUFDDevice *idev;
>> +
>> +if (base_dev->type == HID_LEGACY) {
>> +IOMMULegacyDevice *ldev = container_of(base_dev,
>> + IOMMULegacyDevice, base);
>> +
>> +return vtd_check_legacy_hdev(s, ldev, errp);
>> +}
>> +
>> +idev = container_of(base_dev, IOMMUFDDevice, base);
>> +
>> +return vtd_check_iommufd_hdev(s, idev, errp);
>> +}
>> +
>> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
> devfn,
>> HostIOMMUDevice *base_dev, Error
>> **errp)
>> {
>> @@ -3829,6 +3863,7 @@ static int
>>> vtd_dev_set_iommu_device(PCIBus
> *bus, void *opaque, int devfn,
>> .devfn = devfn,
>> };
>> struct vtd_as_key *new_key;
>> +int ret;
>>
>> assert(base_dev);
>>
>> @@ -3848,6 +3883,13 @@ static int
>>> vtd_dev_set_iommu_device(PCIBus
> *bus, void *opaque, int devfn,
>> vtd_hdev->iommu_state = s;
>> vtd_hdev->dev = base_dev;
>>
>> +ret = vtd_check_hdev(s, vtd_hdev, errp);
>> +if (ret) {
>> +g_free(vtd_hdev);
>> +vtd_iommu_unlock(s);
>> +return ret;
>> +}
>> +
>> new_key = g_malloc(sizeof(*new_key));
>> new_key->bus = bus;
>> new_key->devfn = devfn;
>
> Okay. So when VFIO device is created, it will call
>>> vtd_dev_set_iommu_device
> and that in turn will update caps.
>
>
>
RE: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
>-Original Message-
>From: Michael S. Tsirkin
>Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>sync host IOMMU cap/ecap
>
>On Wed, Mar 13, 2024 at 07:54:11AM +, Duan, Zhenzhong wrote:
>>
>>
>> >-Original Message-
>> >From: Michael S. Tsirkin
>> >Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>> >sync host IOMMU cap/ecap
>> >
>> >On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
>> >> Hi Michael,
>> >>
>> >> >-Original Message-
>> >> >From: Michael S. Tsirkin
>> >> >Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check
>and
>> >> >sync host IOMMU cap/ecap
>> >> >
>> >> >On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
>> >> >> From: Yi Liu
>> >> >>
>> >> >> Add a framework to check and synchronize host IOMMU cap/ecap
>with
>> >> >> vIOMMU cap/ecap.
>> >> >>
>> >> >> The sequence will be:
>> >> >>
>> >> >> vtd_cap_init() initializes iommu->cap/ecap.
>> >> >> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
>> >> >> iommu->cap_frozen set when machine create done, iommu-
>>cap/ecap
>> >> >become readonly.
>> >> >>
>> >> >> Implementation details for different backends will be in following
>> >patches.
>> >> >>
>> >> >> Signed-off-by: Yi Liu
>> >> >> Signed-off-by: Yi Sun
>> >> >> Signed-off-by: Zhenzhong Duan
>> >> >> ---
>> >> >> include/hw/i386/intel_iommu.h | 1 +
>> >> >> hw/i386/intel_iommu.c | 50
>> >> >++-
>> >> >> 2 files changed, 50 insertions(+), 1 deletion(-)
>> >> >>
>> >> >> diff --git a/include/hw/i386/intel_iommu.h
>> >> >b/include/hw/i386/intel_iommu.h
>> >> >> index bbc7b96add..c71a133820 100644
>> >> >> --- a/include/hw/i386/intel_iommu.h
>> >> >> +++ b/include/hw/i386/intel_iommu.h
>> >> >> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
>> >> >>
>> >> >> uint64_t cap; /* The value of capability reg */
>> >> >> uint64_t ecap; /* The value of extended
>> >> >> capability reg
>*/
>> >> >> +bool cap_frozen;/* cap/ecap become read-only after
>> >frozen */
>> >> >>
>> >> >> uint32_t context_cache_gen; /* Should be in [1,MAX] */
>> >> >> GHashTable *iotlb; /* IOTLB */
>> >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> >> >> index ffa1ad6429..a9f9dfd6a7 100644
>> >> >> --- a/hw/i386/intel_iommu.c
>> >> >> +++ b/hw/i386/intel_iommu.c
>> >> >> @@ -35,6 +35,8 @@
>> >> >> #include "sysemu/kvm.h"
>> >> >> #include "sysemu/dma.h"
>> >> >> #include "sysemu/sysemu.h"
>> >> >> +#include "hw/vfio/vfio-common.h"
>> >> >> +#include "sysemu/iommufd.h"
>> >> >> #include "hw/i386/apic_internal.h"
>> >> >> #include "kvm/kvm_i386.h"
>> >> >> #include "migration/vmstate.h"
>> >> >> @@ -3819,6 +3821,38 @@ VTDAddressSpace
>> >> >*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>> >> >> return vtd_dev_as;
>> >> >> }
>> >> >>
>> >> >> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>> >> >> + IOMMULegacyDevice *ldev,
>> >> >> + Error **errp)
>> >> >> +{
>> >> >> +return 0;
>> >> >> +}
>> >> >> +
>> >> >> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>> >> >> + IOMMUFDDevice *idev,
>> >> >> + Error **errp)
>> >> >> +{
>> >> >> +return 0;
>> >> >> +}
>> >> >> +
>> >> >> +static int vtd_check_hdev(IntelIOMMUState *s,
>> >VTDHostIOMMUDevice
>> >> >*vtd_hdev,
>> >> >> + Error **errp)
>> >> >> +{
>> >> >> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
>> >> >> +IOMMUFDDevice *idev;
>> >> >> +
>> >> >> +if (base_dev->type == HID_LEGACY) {
>> >> >> +IOMMULegacyDevice *ldev = container_of(base_dev,
>> >> >> + IOMMULegacyDevice,
>> >> >> base);
>> >> >> +
>> >> >> +return vtd_check_legacy_hdev(s, ldev, errp);
>> >> >> +}
>> >> >> +
>> >> >> +idev = container_of(base_dev, IOMMUFDDevice, base);
>> >> >> +
>> >> >> +return vtd_check_iommufd_hdev(s, idev, errp);
>> >> >> +}
>> >> >> +
>> >> >> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque,
>int
>> >> >devfn,
>> >> >> HostIOMMUDevice *base_dev, Error
>> >> >> **errp)
>> >> >> {
>> >> >> @@ -3829,6 +3863,7 @@ static int
>> >vtd_dev_set_iommu_device(PCIBus
>> >> >*bus, void *opaque, int devfn,
>> >> >> .devfn = devfn,
>> >> >> };
>> >> >> struct vtd_as_key *new_key;
>> >> >> +int ret;
>> >> >>
>> >> >> assert(base_dev);
>> >> >>
>> >> >> @@ -3848,6 +3883,13 @@ static int
>> >vtd_dev_set_iommu_device(PCIBus
>> >> >*bus, void *opaque, int devfn,
>> >> >> vtd_hdev->iommu_state = s;
>> >> >> vtd_hdev->dev = base_dev;
>> >> >>
>> >> >> +ret = vtd_check_hdev(s, vtd_hdev, errp);
>> >> >> +if (ret) {
>> >> >> +g_free(vtd_hdev);
Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
On Wed, Mar 13, 2024 at 07:54:11AM +, Duan, Zhenzhong wrote:
>
>
> >-Original Message-
> >From: Michael S. Tsirkin
> >Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
> >sync host IOMMU cap/ecap
> >
> >On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
> >> Hi Michael,
> >>
> >> >-Original Message-
> >> >From: Michael S. Tsirkin
> >> >Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
> >> >sync host IOMMU cap/ecap
> >> >
> >> >On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
> >> >> From: Yi Liu
> >> >>
> >> >> Add a framework to check and synchronize host IOMMU cap/ecap with
> >> >> vIOMMU cap/ecap.
> >> >>
> >> >> The sequence will be:
> >> >>
> >> >> vtd_cap_init() initializes iommu->cap/ecap.
> >> >> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
> >> >> iommu->cap_frozen set when machine create done, iommu->cap/ecap
> >> >become readonly.
> >> >>
> >> >> Implementation details for different backends will be in following
> >patches.
> >> >>
> >> >> Signed-off-by: Yi Liu
> >> >> Signed-off-by: Yi Sun
> >> >> Signed-off-by: Zhenzhong Duan
> >> >> ---
> >> >> include/hw/i386/intel_iommu.h | 1 +
> >> >> hw/i386/intel_iommu.c | 50
> >> >++-
> >> >> 2 files changed, 50 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/include/hw/i386/intel_iommu.h
> >> >b/include/hw/i386/intel_iommu.h
> >> >> index bbc7b96add..c71a133820 100644
> >> >> --- a/include/hw/i386/intel_iommu.h
> >> >> +++ b/include/hw/i386/intel_iommu.h
> >> >> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
> >> >>
> >> >> uint64_t cap; /* The value of capability reg */
> >> >> uint64_t ecap; /* The value of extended
> >> >> capability reg */
> >> >> +bool cap_frozen;/* cap/ecap become read-only after
> >frozen */
> >> >>
> >> >> uint32_t context_cache_gen; /* Should be in [1,MAX] */
> >> >> GHashTable *iotlb; /* IOTLB */
> >> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> >> >> index ffa1ad6429..a9f9dfd6a7 100644
> >> >> --- a/hw/i386/intel_iommu.c
> >> >> +++ b/hw/i386/intel_iommu.c
> >> >> @@ -35,6 +35,8 @@
> >> >> #include "sysemu/kvm.h"
> >> >> #include "sysemu/dma.h"
> >> >> #include "sysemu/sysemu.h"
> >> >> +#include "hw/vfio/vfio-common.h"
> >> >> +#include "sysemu/iommufd.h"
> >> >> #include "hw/i386/apic_internal.h"
> >> >> #include "kvm/kvm_i386.h"
> >> >> #include "migration/vmstate.h"
> >> >> @@ -3819,6 +3821,38 @@ VTDAddressSpace
> >> >*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
> >> >> return vtd_dev_as;
> >> >> }
> >> >>
> >> >> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
> >> >> + IOMMULegacyDevice *ldev,
> >> >> + Error **errp)
> >> >> +{
> >> >> +return 0;
> >> >> +}
> >> >> +
> >> >> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
> >> >> + IOMMUFDDevice *idev,
> >> >> + Error **errp)
> >> >> +{
> >> >> +return 0;
> >> >> +}
> >> >> +
> >> >> +static int vtd_check_hdev(IntelIOMMUState *s,
> >VTDHostIOMMUDevice
> >> >*vtd_hdev,
> >> >> + Error **errp)
> >> >> +{
> >> >> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
> >> >> +IOMMUFDDevice *idev;
> >> >> +
> >> >> +if (base_dev->type == HID_LEGACY) {
> >> >> +IOMMULegacyDevice *ldev = container_of(base_dev,
> >> >> + IOMMULegacyDevice,
> >> >> base);
> >> >> +
> >> >> +return vtd_check_legacy_hdev(s, ldev, errp);
> >> >> +}
> >> >> +
> >> >> +idev = container_of(base_dev, IOMMUFDDevice, base);
> >> >> +
> >> >> +return vtd_check_iommufd_hdev(s, idev, errp);
> >> >> +}
> >> >> +
> >> >> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
> >> >devfn,
> >> >> HostIOMMUDevice *base_dev, Error
> >> >> **errp)
> >> >> {
> >> >> @@ -3829,6 +3863,7 @@ static int
> >vtd_dev_set_iommu_device(PCIBus
> >> >*bus, void *opaque, int devfn,
> >> >> .devfn = devfn,
> >> >> };
> >> >> struct vtd_as_key *new_key;
> >> >> +int ret;
> >> >>
> >> >> assert(base_dev);
> >> >>
> >> >> @@ -3848,6 +3883,13 @@ static int
> >vtd_dev_set_iommu_device(PCIBus
> >> >*bus, void *opaque, int devfn,
> >> >> vtd_hdev->iommu_state = s;
> >> >> vtd_hdev->dev = base_dev;
> >> >>
> >> >> +ret = vtd_check_hdev(s, vtd_hdev, errp);
> >> >> +if (ret) {
> >> >> +g_free(vtd_hdev);
> >> >> +vtd_iommu_unlock(s);
> >> >> +return ret;
> >> >> +}
> >> >> +
> >> >> new_key = g_malloc(sizeof(*new_key));
> >> >> new_key->bus = bus;
> >> >> new_key->devfn = devfn;
> >> >
> >> >
> >> >Okay. So when VFIO device is created, it will call
>
RE: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
>-Original Message-
>From: Michael S. Tsirkin
>Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>sync host IOMMU cap/ecap
>
>On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
>> Hi Michael,
>>
>> >-Original Message-
>> >From: Michael S. Tsirkin
>> >Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>> >sync host IOMMU cap/ecap
>> >
>> >On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
>> >> From: Yi Liu
>> >>
>> >> Add a framework to check and synchronize host IOMMU cap/ecap with
>> >> vIOMMU cap/ecap.
>> >>
>> >> The sequence will be:
>> >>
>> >> vtd_cap_init() initializes iommu->cap/ecap.
>> >> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
>> >> iommu->cap_frozen set when machine create done, iommu->cap/ecap
>> >become readonly.
>> >>
>> >> Implementation details for different backends will be in following
>patches.
>> >>
>> >> Signed-off-by: Yi Liu
>> >> Signed-off-by: Yi Sun
>> >> Signed-off-by: Zhenzhong Duan
>> >> ---
>> >> include/hw/i386/intel_iommu.h | 1 +
>> >> hw/i386/intel_iommu.c | 50
>> >++-
>> >> 2 files changed, 50 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/include/hw/i386/intel_iommu.h
>> >b/include/hw/i386/intel_iommu.h
>> >> index bbc7b96add..c71a133820 100644
>> >> --- a/include/hw/i386/intel_iommu.h
>> >> +++ b/include/hw/i386/intel_iommu.h
>> >> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
>> >>
>> >> uint64_t cap; /* The value of capability reg */
>> >> uint64_t ecap; /* The value of extended capability
>> >> reg */
>> >> +bool cap_frozen;/* cap/ecap become read-only after
>frozen */
>> >>
>> >> uint32_t context_cache_gen; /* Should be in [1,MAX] */
>> >> GHashTable *iotlb; /* IOTLB */
>> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> >> index ffa1ad6429..a9f9dfd6a7 100644
>> >> --- a/hw/i386/intel_iommu.c
>> >> +++ b/hw/i386/intel_iommu.c
>> >> @@ -35,6 +35,8 @@
>> >> #include "sysemu/kvm.h"
>> >> #include "sysemu/dma.h"
>> >> #include "sysemu/sysemu.h"
>> >> +#include "hw/vfio/vfio-common.h"
>> >> +#include "sysemu/iommufd.h"
>> >> #include "hw/i386/apic_internal.h"
>> >> #include "kvm/kvm_i386.h"
>> >> #include "migration/vmstate.h"
>> >> @@ -3819,6 +3821,38 @@ VTDAddressSpace
>> >*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>> >> return vtd_dev_as;
>> >> }
>> >>
>> >> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>> >> + IOMMULegacyDevice *ldev,
>> >> + Error **errp)
>> >> +{
>> >> +return 0;
>> >> +}
>> >> +
>> >> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>> >> + IOMMUFDDevice *idev,
>> >> + Error **errp)
>> >> +{
>> >> +return 0;
>> >> +}
>> >> +
>> >> +static int vtd_check_hdev(IntelIOMMUState *s,
>VTDHostIOMMUDevice
>> >*vtd_hdev,
>> >> + Error **errp)
>> >> +{
>> >> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
>> >> +IOMMUFDDevice *idev;
>> >> +
>> >> +if (base_dev->type == HID_LEGACY) {
>> >> +IOMMULegacyDevice *ldev = container_of(base_dev,
>> >> + IOMMULegacyDevice, base);
>> >> +
>> >> +return vtd_check_legacy_hdev(s, ldev, errp);
>> >> +}
>> >> +
>> >> +idev = container_of(base_dev, IOMMUFDDevice, base);
>> >> +
>> >> +return vtd_check_iommufd_hdev(s, idev, errp);
>> >> +}
>> >> +
>> >> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
>> >devfn,
>> >> HostIOMMUDevice *base_dev, Error
>> >> **errp)
>> >> {
>> >> @@ -3829,6 +3863,7 @@ static int
>vtd_dev_set_iommu_device(PCIBus
>> >*bus, void *opaque, int devfn,
>> >> .devfn = devfn,
>> >> };
>> >> struct vtd_as_key *new_key;
>> >> +int ret;
>> >>
>> >> assert(base_dev);
>> >>
>> >> @@ -3848,6 +3883,13 @@ static int
>vtd_dev_set_iommu_device(PCIBus
>> >*bus, void *opaque, int devfn,
>> >> vtd_hdev->iommu_state = s;
>> >> vtd_hdev->dev = base_dev;
>> >>
>> >> +ret = vtd_check_hdev(s, vtd_hdev, errp);
>> >> +if (ret) {
>> >> +g_free(vtd_hdev);
>> >> +vtd_iommu_unlock(s);
>> >> +return ret;
>> >> +}
>> >> +
>> >> new_key = g_malloc(sizeof(*new_key));
>> >> new_key->bus = bus;
>> >> new_key->devfn = devfn;
>> >
>> >
>> >Okay. So when VFIO device is created, it will call
>vtd_dev_set_iommu_device
>> >and that in turn will update caps.
>> >
>> >
>> >
>> >
>> >> @@ -4083,7 +4125,9 @@ static void vtd_init(IntelIOMMUState *s)
>> >> s->iq_dw = false;
>> >> s->next_frcd_reg = 0;
>> >>
>> >> -vtd_cap_init(s);
>> >> +if (!s->cap_frozen) {
>> >> +vtd_cap_init(s);
>> >> +}
>> >>
>> >
>> >If it's fronzen it's
Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
On Wed, Mar 13, 2024 at 02:52:39AM +, Duan, Zhenzhong wrote:
> Hi Michael,
>
> >-Original Message-
> >From: Michael S. Tsirkin
> >Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
> >sync host IOMMU cap/ecap
> >
> >On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
> >> From: Yi Liu
> >>
> >> Add a framework to check and synchronize host IOMMU cap/ecap with
> >> vIOMMU cap/ecap.
> >>
> >> The sequence will be:
> >>
> >> vtd_cap_init() initializes iommu->cap/ecap.
> >> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
> >> iommu->cap_frozen set when machine create done, iommu->cap/ecap
> >become readonly.
> >>
> >> Implementation details for different backends will be in following patches.
> >>
> >> Signed-off-by: Yi Liu
> >> Signed-off-by: Yi Sun
> >> Signed-off-by: Zhenzhong Duan
> >> ---
> >> include/hw/i386/intel_iommu.h | 1 +
> >> hw/i386/intel_iommu.c | 50
> >++-
> >> 2 files changed, 50 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/include/hw/i386/intel_iommu.h
> >b/include/hw/i386/intel_iommu.h
> >> index bbc7b96add..c71a133820 100644
> >> --- a/include/hw/i386/intel_iommu.h
> >> +++ b/include/hw/i386/intel_iommu.h
> >> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
> >>
> >> uint64_t cap; /* The value of capability reg */
> >> uint64_t ecap; /* The value of extended capability
> >> reg */
> >> +bool cap_frozen;/* cap/ecap become read-only after
> >> frozen */
> >>
> >> uint32_t context_cache_gen; /* Should be in [1,MAX] */
> >> GHashTable *iotlb; /* IOTLB */
> >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> >> index ffa1ad6429..a9f9dfd6a7 100644
> >> --- a/hw/i386/intel_iommu.c
> >> +++ b/hw/i386/intel_iommu.c
> >> @@ -35,6 +35,8 @@
> >> #include "sysemu/kvm.h"
> >> #include "sysemu/dma.h"
> >> #include "sysemu/sysemu.h"
> >> +#include "hw/vfio/vfio-common.h"
> >> +#include "sysemu/iommufd.h"
> >> #include "hw/i386/apic_internal.h"
> >> #include "kvm/kvm_i386.h"
> >> #include "migration/vmstate.h"
> >> @@ -3819,6 +3821,38 @@ VTDAddressSpace
> >*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
> >> return vtd_dev_as;
> >> }
> >>
> >> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
> >> + IOMMULegacyDevice *ldev,
> >> + Error **errp)
> >> +{
> >> +return 0;
> >> +}
> >> +
> >> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
> >> + IOMMUFDDevice *idev,
> >> + Error **errp)
> >> +{
> >> +return 0;
> >> +}
> >> +
> >> +static int vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice
> >*vtd_hdev,
> >> + Error **errp)
> >> +{
> >> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
> >> +IOMMUFDDevice *idev;
> >> +
> >> +if (base_dev->type == HID_LEGACY) {
> >> +IOMMULegacyDevice *ldev = container_of(base_dev,
> >> + IOMMULegacyDevice, base);
> >> +
> >> +return vtd_check_legacy_hdev(s, ldev, errp);
> >> +}
> >> +
> >> +idev = container_of(base_dev, IOMMUFDDevice, base);
> >> +
> >> +return vtd_check_iommufd_hdev(s, idev, errp);
> >> +}
> >> +
> >> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
> >devfn,
> >> HostIOMMUDevice *base_dev, Error
> >> **errp)
> >> {
> >> @@ -3829,6 +3863,7 @@ static int vtd_dev_set_iommu_device(PCIBus
> >*bus, void *opaque, int devfn,
> >> .devfn = devfn,
> >> };
> >> struct vtd_as_key *new_key;
> >> +int ret;
> >>
> >> assert(base_dev);
> >>
> >> @@ -3848,6 +3883,13 @@ static int vtd_dev_set_iommu_device(PCIBus
> >*bus, void *opaque, int devfn,
> >> vtd_hdev->iommu_state = s;
> >> vtd_hdev->dev = base_dev;
> >>
> >> +ret = vtd_check_hdev(s, vtd_hdev, errp);
> >> +if (ret) {
> >> +g_free(vtd_hdev);
> >> +vtd_iommu_unlock(s);
> >> +return ret;
> >> +}
> >> +
> >> new_key = g_malloc(sizeof(*new_key));
> >> new_key->bus = bus;
> >> new_key->devfn = devfn;
> >
> >
> >Okay. So when VFIO device is created, it will call vtd_dev_set_iommu_device
> >and that in turn will update caps.
> >
> >
> >
> >
> >> @@ -4083,7 +4125,9 @@ static void vtd_init(IntelIOMMUState *s)
> >> s->iq_dw = false;
> >> s->next_frcd_reg = 0;
> >>
> >> -vtd_cap_init(s);
> >> +if (!s->cap_frozen) {
> >> +vtd_cap_init(s);
> >> +}
> >>
> >
> >If it's fronzen it's because VFIO was added after machine done.
> >And then what? I think caps are just wrong?
>
> Not quite get your question on caps being wrong. But try to explains:
>
> When a hot plugged vfio device's host iommu cap isn't compatible with
> vIOMMU's, hotplug should fail. Currently there is no check
RE: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
Hi Michael,
>-Original Message-
>From: Michael S. Tsirkin
>Subject: Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and
>sync host IOMMU cap/ecap
>
>On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
>> From: Yi Liu
>>
>> Add a framework to check and synchronize host IOMMU cap/ecap with
>> vIOMMU cap/ecap.
>>
>> The sequence will be:
>>
>> vtd_cap_init() initializes iommu->cap/ecap.
>> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
>> iommu->cap_frozen set when machine create done, iommu->cap/ecap
>become readonly.
>>
>> Implementation details for different backends will be in following patches.
>>
>> Signed-off-by: Yi Liu
>> Signed-off-by: Yi Sun
>> Signed-off-by: Zhenzhong Duan
>> ---
>> include/hw/i386/intel_iommu.h | 1 +
>> hw/i386/intel_iommu.c | 50
>++-
>> 2 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/i386/intel_iommu.h
>b/include/hw/i386/intel_iommu.h
>> index bbc7b96add..c71a133820 100644
>> --- a/include/hw/i386/intel_iommu.h
>> +++ b/include/hw/i386/intel_iommu.h
>> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
>>
>> uint64_t cap; /* The value of capability reg */
>> uint64_t ecap; /* The value of extended capability reg
>> */
>> +bool cap_frozen;/* cap/ecap become read-only after
>> frozen */
>>
>> uint32_t context_cache_gen; /* Should be in [1,MAX] */
>> GHashTable *iotlb; /* IOTLB */
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index ffa1ad6429..a9f9dfd6a7 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -35,6 +35,8 @@
>> #include "sysemu/kvm.h"
>> #include "sysemu/dma.h"
>> #include "sysemu/sysemu.h"
>> +#include "hw/vfio/vfio-common.h"
>> +#include "sysemu/iommufd.h"
>> #include "hw/i386/apic_internal.h"
>> #include "kvm/kvm_i386.h"
>> #include "migration/vmstate.h"
>> @@ -3819,6 +3821,38 @@ VTDAddressSpace
>*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
>> return vtd_dev_as;
>> }
>>
>> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>> + IOMMULegacyDevice *ldev,
>> + Error **errp)
>> +{
>> +return 0;
>> +}
>> +
>> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>> + IOMMUFDDevice *idev,
>> + Error **errp)
>> +{
>> +return 0;
>> +}
>> +
>> +static int vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice
>*vtd_hdev,
>> + Error **errp)
>> +{
>> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
>> +IOMMUFDDevice *idev;
>> +
>> +if (base_dev->type == HID_LEGACY) {
>> +IOMMULegacyDevice *ldev = container_of(base_dev,
>> + IOMMULegacyDevice, base);
>> +
>> +return vtd_check_legacy_hdev(s, ldev, errp);
>> +}
>> +
>> +idev = container_of(base_dev, IOMMUFDDevice, base);
>> +
>> +return vtd_check_iommufd_hdev(s, idev, errp);
>> +}
>> +
>> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int
>devfn,
>> HostIOMMUDevice *base_dev, Error **errp)
>> {
>> @@ -3829,6 +3863,7 @@ static int vtd_dev_set_iommu_device(PCIBus
>*bus, void *opaque, int devfn,
>> .devfn = devfn,
>> };
>> struct vtd_as_key *new_key;
>> +int ret;
>>
>> assert(base_dev);
>>
>> @@ -3848,6 +3883,13 @@ static int vtd_dev_set_iommu_device(PCIBus
>*bus, void *opaque, int devfn,
>> vtd_hdev->iommu_state = s;
>> vtd_hdev->dev = base_dev;
>>
>> +ret = vtd_check_hdev(s, vtd_hdev, errp);
>> +if (ret) {
>> +g_free(vtd_hdev);
>> +vtd_iommu_unlock(s);
>> +return ret;
>> +}
>> +
>> new_key = g_malloc(sizeof(*new_key));
>> new_key->bus = bus;
>> new_key->devfn = devfn;
>
>
>Okay. So when VFIO device is created, it will call vtd_dev_set_iommu_device
>and that in turn will update caps.
>
>
>
>
>> @@ -4083,7 +4125,9 @@ static void vtd_init(IntelIOMMUState *s)
>> s->iq_dw = false;
>> s->next_frcd_reg = 0;
>>
>> -vtd_cap_init(s);
>> +if (!s->cap_frozen) {
>> +vtd_cap_init(s);
>> +}
>>
>
>If it's fronzen it's because VFIO was added after machine done.
>And then what? I think caps are just wrong?
Not quite get your question on caps being wrong. But try to explains:
When a hot plugged vfio device's host iommu cap isn't compatible with
vIOMMU's, hotplug should fail. Currently there is no check for this and
allow hotplug to succeed, but then some issue will reveal later,
e.g., vIOMMU's MGAW > host IOMMU's MGAW, guest can setup iova
mapping beyond host supported iova range, then DMA will fail.
In fact, before this series, cap is not impacted by VFIO, so it's same effect of
frozen after machine done.
>
>
>I think the way to approach this is just by specifying
Re: [PATCH v1 3/6] intel_iommu: Add a framework to check and sync host IOMMU cap/ecap
On Wed, Feb 28, 2024 at 05:44:29PM +0800, Zhenzhong Duan wrote:
> From: Yi Liu
>
> Add a framework to check and synchronize host IOMMU cap/ecap with
> vIOMMU cap/ecap.
>
> The sequence will be:
>
> vtd_cap_init() initializes iommu->cap/ecap.
> vtd_check_hdev() update iommu->cap/ecap based on host cap/ecap.
> iommu->cap_frozen set when machine create done, iommu->cap/ecap become
> readonly.
>
> Implementation details for different backends will be in following patches.
>
> Signed-off-by: Yi Liu
> Signed-off-by: Yi Sun
> Signed-off-by: Zhenzhong Duan
> ---
> include/hw/i386/intel_iommu.h | 1 +
> hw/i386/intel_iommu.c | 50 ++-
> 2 files changed, 50 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
> index bbc7b96add..c71a133820 100644
> --- a/include/hw/i386/intel_iommu.h
> +++ b/include/hw/i386/intel_iommu.h
> @@ -283,6 +283,7 @@ struct IntelIOMMUState {
>
> uint64_t cap; /* The value of capability reg */
> uint64_t ecap; /* The value of extended capability reg
> */
> +bool cap_frozen;/* cap/ecap become read-only after
> frozen */
>
> uint32_t context_cache_gen; /* Should be in [1,MAX] */
> GHashTable *iotlb; /* IOTLB */
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index ffa1ad6429..a9f9dfd6a7 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -35,6 +35,8 @@
> #include "sysemu/kvm.h"
> #include "sysemu/dma.h"
> #include "sysemu/sysemu.h"
> +#include "hw/vfio/vfio-common.h"
> +#include "sysemu/iommufd.h"
> #include "hw/i386/apic_internal.h"
> #include "kvm/kvm_i386.h"
> #include "migration/vmstate.h"
> @@ -3819,6 +3821,38 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState *s,
> PCIBus *bus,
> return vtd_dev_as;
> }
>
> +static int vtd_check_legacy_hdev(IntelIOMMUState *s,
> + IOMMULegacyDevice *ldev,
> + Error **errp)
> +{
> +return 0;
> +}
> +
> +static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
> + IOMMUFDDevice *idev,
> + Error **errp)
> +{
> +return 0;
> +}
> +
> +static int vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hdev,
> + Error **errp)
> +{
> +HostIOMMUDevice *base_dev = vtd_hdev->dev;
> +IOMMUFDDevice *idev;
> +
> +if (base_dev->type == HID_LEGACY) {
> +IOMMULegacyDevice *ldev = container_of(base_dev,
> + IOMMULegacyDevice, base);
> +
> +return vtd_check_legacy_hdev(s, ldev, errp);
> +}
> +
> +idev = container_of(base_dev, IOMMUFDDevice, base);
> +
> +return vtd_check_iommufd_hdev(s, idev, errp);
> +}
> +
> static int vtd_dev_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
> HostIOMMUDevice *base_dev, Error **errp)
> {
> @@ -3829,6 +3863,7 @@ static int vtd_dev_set_iommu_device(PCIBus *bus, void
> *opaque, int devfn,
> .devfn = devfn,
> };
> struct vtd_as_key *new_key;
> +int ret;
>
> assert(base_dev);
>
> @@ -3848,6 +3883,13 @@ static int vtd_dev_set_iommu_device(PCIBus *bus, void
> *opaque, int devfn,
> vtd_hdev->iommu_state = s;
> vtd_hdev->dev = base_dev;
>
> +ret = vtd_check_hdev(s, vtd_hdev, errp);
> +if (ret) {
> +g_free(vtd_hdev);
> +vtd_iommu_unlock(s);
> +return ret;
> +}
> +
> new_key = g_malloc(sizeof(*new_key));
> new_key->bus = bus;
> new_key->devfn = devfn;
Okay. So when VFIO device is created, it will call vtd_dev_set_iommu_device
and that in turn will update caps.
> @@ -4083,7 +4125,9 @@ static void vtd_init(IntelIOMMUState *s)
> s->iq_dw = false;
> s->next_frcd_reg = 0;
>
> -vtd_cap_init(s);
> +if (!s->cap_frozen) {
> +vtd_cap_init(s);
> +}
>
If it's fronzen it's because VFIO was added after machine done.
And then what? I think caps are just wrong?
I think the way to approach this is just by specifying this
as an option on command line.
So if one wants VFIO one has to sync caps with host.
No?
> /*
> * Rsvd field masks for spte
> @@ -4254,6 +4298,10 @@ static int vtd_machine_done_notify_one(Object *child,
> void *unused)
>
> static void vtd_machine_done_hook(Notifier *notifier, void *unused)
> {
> +IntelIOMMUState *iommu = INTEL_IOMMU_DEVICE(x86_iommu_get_default());
> +
> +iommu->cap_frozen = true;
> +
> object_child_foreach_recursive(object_get_root(),
> vtd_machine_done_notify_one, NULL);
> }
> --
> 2.34.1
