Re: [PATCH v2 3/3] [FUTURE] xen/arm: enable vPCI for domUs

[Rahul Singh <Rahul.Singh@arm.com>]

Hi Stewart,

> On 21 Jul 2023, at 5:54 am, Stewart Hildebrand <Stewart.Hildebrand@amd.com> wrote:
> 
> On 7/7/23 07:04, Rahul Singh wrote:
>> Hi Stewart,
>> 
>>> On 7 Jul 2023, at 2:47 am, Stewart Hildebrand <Stewart.Hildebrand@amd.com> wrote:
>>> 
>>> Remove is_hardware_domain check in has_vpci, and select HAS_VPCI_GUEST_SUPPORT
>>> in Kconfig.
>>> 
>>> [1] https://lists.xenproject.org/archives/html/xen-devel/2023-06/msg00863.html
>>> 
>>> Signed-off-by: Stewart Hildebrand <stewart.hildebrand@amd.com>
>>> ---
>>> As the tag implies, this patch is not intended to be merged (yet).
>>> 
>>> Note that CONFIG_HAS_VPCI_GUEST_SUPPORT is not currently used in the upstream
>>> code base. It will be used by the vPCI series [1]. This patch is intended to be
>>> merged as part of the vPCI series.
>>> 
>>> v1->v2:
>>> * new patch
>>> ---
>>> xen/arch/arm/Kconfig              | 1 +
>>> xen/arch/arm/include/asm/domain.h | 2 +-
>>> 2 files changed, 2 insertions(+), 1 deletion(-)
>>> 
>>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>>> index 4e0cc421ad48..75dfa2f5a82d 100644
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -195,6 +195,7 @@ config PCI_PASSTHROUGH
>>> depends on ARM_64
>>> select HAS_PCI
>>> select HAS_VPCI
>>> + select HAS_VPCI_GUEST_SUPPORT
>> 
>> I tested this series on top of "SMMU handling for PCIe Passthrough on ARM” series on the N1SDP board
>> and observe the SMMUv3 fault.
> 
> Thanks for testing this. After a great deal of tinkering, I can reproduce the SMMU fault.
> 
> (XEN) smmu: /axi/smmu@fd800000: Unhandled context fault: fsr=0x402, iova=0xf9030040, fsynr=0x12, cb=0
> 
>> Enable the Kconfig option PCI_PASSTHROUGH, ARM_SMMU_V3,HAS_ITS and "iommu=on”,
>> "pci_passthrough_enabled=on" cmd line parameter and after that, there is an SMMU fault
>> for the ITS doorbell register access from the PCI devices.
>> 
>> As there is no upstream support for ARM for vPCI MSI/MSI-X handling because of that SMMU fault is observed.
>> 
>> Linux Kernel will set the ITS doorbell register( physical address of doorbell register as IOMMU is not enabled in Kernel)
>> in PCI config space to set up the MSI-X interrupts, but there is no mapping in SMMU page tables because of that SMMU
>> fault is observed. To fix this we need to map the ITS doorbell register to SMMU page tables to avoid the fault.
>> 
>> We can fix this after setting the mapping for the ITS doorbell offset in the ITS code.
>> 
>> diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
>> index 299b384250..8227a7a74b 100644
>> --- a/xen/arch/arm/vgic-v3-its.c
>> +++ b/xen/arch/arm/vgic-v3-its.c
>> @@ -682,6 +682,18 @@ static int its_handle_mapd(struct virt_its *its, uint64_t *cmdptr)
>>                                          BIT(size, UL), valid);
>>         if ( ret && valid )
>>             return ret;
>> +
>> +        if ( is_iommu_enabled(its->d) ) {
>> +            ret = map_mmio_regions(its->d, gaddr_to_gfn(its->doorbell_address),
>> +                           PFN_UP(ITS_DOORBELL_OFFSET),
>> +                           maddr_to_mfn(its->doorbell_address));
>> +            if ( ret < 0 )
>> +            {
>> +                printk(XENLOG_ERR "GICv3: Map ITS translation register d%d failed.\n",
>> +                        its->d->domain_id);
>> +                return ret;
>> +            }
>> +        }
>>     }
> 
> Thank you, this resolves the SMMU fault. If it's okay, I will include this patch in the next revision of the SMMU series (I see your Signed-off-by is already in the attachment).

Yes, you can include this patch in your next version.
> 
>> Also as per Julien's request, I tried to set up the IOMMU for the PCI device without
>> "pci_passthroigh_enable=on" and without HAS_VPCI everything works as expected
>> after applying below patches.
>> 
>> To test enable kconfig options HAS_PCI, ARM_SMMU_V3 and HAS_ITS and add below
>> patches to make it work.
>> 
>>    • Set the mapping for the ITS doorbell offset in the ITS code when iommu is enabled.

Also, If we want to support for adding PCI devices to IOMMU without PCI passthrough
support (without HAS_VPCI and cmd line options “pci-passthrough-enabled=on”)
as suggested by Julien, we also need below 2 patches also.

>>    • Reverted the patch that added the support for pci_passthrough_on.
>>    • Allow MMIO mapping of ECAM space to dom0 when vPCI is not enabled, as of now MMIO
>>      mapping for ECAM is based on pci_passthrough_enabled. We need this patch if we want to avoid
>>     enabling HAS_VPCI
>> 
>> Please find the attached patches in case you want to test at your end.
>> 

 
Regards,
Rahul