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

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Stewart,

> On 21 Jul 2023, at 5:54 am, Stewart Hildebrand <Stewart.Hildebrand@xxxxxxx> 
> wrote:
> 
> On 7/7/23 07:04, Rahul Singh wrote:
>> Hi Stewart,
>> 
>>> On 7 Jul 2023, at 2:47 am, Stewart Hildebrand <Stewart.Hildebrand@xxxxxxx> 
>>> 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@xxxxxxx>
>>> ---
>>> 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