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