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

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

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).

> 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.
>     • 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
> 
>> default n
>> help
>>  This option enables PCI device passthrough
>> diff --git a/xen/arch/arm/include/asm/domain.h 
>> b/xen/arch/arm/include/asm/domain.h
>> index 1a13965a26b8..6e016b00bae1 100644
>> --- a/xen/arch/arm/include/asm/domain.h
>> +++ b/xen/arch/arm/include/asm/domain.h
>> @@ -298,7 +298,7 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
>>
>> #define arch_vm_assist_valid_mask(d) (1UL << 
>> VMASST_TYPE_runstate_update_flag)
>>
>> -#define has_vpci(d) ({ IS_ENABLED(CONFIG_HAS_VPCI) && 
>> is_hardware_domain(d); })
>> +#define has_vpci(d)    ({ (void)(d); IS_ENABLED(CONFIG_HAS_VPCI); })
>>
>> struct arch_vcpu_io {
>>     struct instr_details dabt_instr; /* when the instruction is decoded */
>> --
>> 2.41.0
>>
>>
>