[PATCH v2 2/8] iommu/arm-smmu-v3: Use S2FWB when available

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Jason Gunthorpe <jgg at nvidia.com>
> Sent: Wednesday, September 4, 2024 4:00 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi at huawei.com>
> Cc: acpica-devel at lists.linux.dev; Guohanjun (Hanjun Guo)
> <guohanjun at huawei.com>; iommu at lists.linux.dev; Joerg Roedel
> <joro at 8bytes.org>; Kevin Tian <kevin.tian at intel.com>; kvm at vger.kernel.org;
> Len Brown <lenb at kernel.org>; linux-acpi at vger.kernel.org; linux-arm-
> kernel at lists.infradead.org; Lorenzo Pieralisi <lpieralisi at kernel.org>; Rafael J.
> Wysocki <rafael at kernel.org>; Robert Moore <robert.moore at intel.com>; Robin
> Murphy <robin.murphy at arm.com>; Sudeep Holla <sudeep.holla at arm.com>;
> Will Deacon <will at kernel.org>; Alex Williamson
> <alex.williamson at redhat.com>; Eric Auger <eric.auger at redhat.com>; Jean-
> Philippe Brucker <jean-philippe at linaro.org>; Moritz Fischer <mdf at kernel.org>;
> Michael Shavit <mshavit at google.com>; Nicolin Chen <nicolinc at nvidia.com>;
> patches at lists.linux.dev; Mostafa Saleh <smostafa at google.com>
> Subject: Re: [PATCH v2 2/8] iommu/arm-smmu-v3: Use S2FWB when available
> 
> On Wed, Sep 04, 2024 at 02:20:36PM +0000, Shameerali Kolothum Thodi wrote:
> 
> > This should be added to arm_64_lpae_alloc_pgtable_s2(), not here.
> 
> Woops! Yes:
> 
> -       /* The NS quirk doesn't apply at stage 2 */
> -       if (cfg->quirks)
> +       if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_S2FWB))
>                 return NULL;
> 
> > With the above fixed, I was able to assign a n/w VF dev to a Guest on
> > a test hardware that supports S2FWB.
> 
> Okay great
> 
> > However host kernel has this WARN message:
> > [ 1546.165105] WARNING: CPU: 5 PID: 7047 at
> > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:1086
> > arm_smmu_entry_qword_diff+0x124/0x138
> > ....
> 
> Yes, my dumb mistake again, thanks for testing
> 
> @@ -1009,7 +1009,8 @@ void arm_smmu_get_ste_used(const __le64 *ent,
> __le64 *used_bits)
>         /* S2 translates */
>         if (cfg & BIT(1)) {
>                 used_bits[1] |=
> -                       cpu_to_le64(STRTAB_STE_1_EATS | STRTAB_STE_1_SHCFG);
> +                       cpu_to_le64(STRTAB_STE_1_S2FWB | STRTAB_STE_1_EATS |
> +                                   STRTAB_STE_1_SHCFG);
> 
> > root at localhost:/# ping 150.0.124.42
> > PING 150.0.124.42 (150.0.124.42): 56 data bytes
> > 64 bytes from 150.0.124.42: seq=0 ttl=64 time=47.648 ms
> 
> So DMA is not totally broken if a packet flowed.
> 
> > [ 1395.958630] hns3 0000:c2:00.0 eth1: NETDEV WATCHDOG: CPU: 1:
> > transmit queue 10 timed out 5260 ms
> 
> Timeout? Maybe interrupts are not working? Does /proc/interrupts suggest
> that? That would point at the ITS mapping

Interrupt seems to be Ok in this case as I can see /proc/interrupts increasing.

> Do you have all of Nicolin's extra patches in this kernel to make the ITS work
> with nesting?

Yes. I am using his
 https://github.com/nicolinc/iommufd/commits/iommufd_viommu_p1-v2/

> From a page table POV, iommu_dma_get_msi_page() has:
> 
> 	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
> 
> So the ITS page should be:
> 
> 		if (prot & IOMMU_MMIO) {
> 			pte |= ARM_LPAE_PTE_MEMATTR_DEV;
> 
> Which which still looks right under S2FWB unless I've misread the manual?
> 
> > [ 1395.960187] hns3 0000:c2:00.0 eth1: DQL info last_cnt: 42, queued:
> > 42, adj_limit: 0, completed: 0 [ 1395.961758] hns3 0000:c2:00.0 eth1:
> > queue state: 0x6, delta msecs: 5260 [ 1395.962925] hns3 0000:c2:00.0
> > eth1: tx_timeout count: 1, queue id: 10, SW_NTU: 0x1, SW_NTC: 0x0,
> > napi state: 16 [ 1395.964677] hns3 0000:c2:00.0 eth1: tx_pkts: 0,
> > tx_bytes: 0, sw_err_cnt: 0, tx_pending: 0 [ 1395.966114] hns3
> > 0000:c2:00.0 eth1: seg_pkt_cnt: 0, tx_more: 0, restart_queue: 0,
> > tx_busy: 0 [ 1395.967598] hns3 0000:c2:00.0 eth1: tx_push: 1,
> > tx_mem_doorbell: 0 [ 1395.968687] hns3 0000:c2:00.0 eth1: BD_NUM: 0x7f
> > HW_HEAD: 0x0, HW_TAIL: 0x0, BD_ERR: 0x0, INT: 0x1 [ 1395.970291] hns3
> > 0000:c2:00.0 eth1: RING_EN: 0x1, TC: 0x0, FBD_NUM: 0x0 FBD_OFT: 0x0,
> > EBD_NUM: 0x400, EBD_OFT: 0x0 [ 1395.972134] hns3 0000:c2:00.0:
> > received reset request from VF enet
> >
> > All this works fine on a hardware without S2FWB though.
> >
> > Also on this test hardware, it works fine with legacy VFIO assignment.
> 
> So.. Legacy VFIO assignment will use the S1, no nesting and not enable S2FWB?

Yes S1
 
> Try to isolate if S2FWB is the exact cause by disabling it in the kernel on this
> system vs something else wrong?

It looks like not related to S2FWB. I tried  commenting out S2FWB and issue is still
there.  Probably something related to this test setup.

Thanks,
Shameer