Re: [RFC PATCH 24/45] KVM: arm64: smmu-v3: Setup stream table

Mostafa Saleh
2024-02-16T12:19:27Z

On Tue, Jan 23, 2024 at 7:45 PM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> Hi Mostafa,
>
> On Tue, Jan 16, 2024 at 08:59:41AM +0000, Mostafa Saleh wrote:
> > > +__maybe_unused
> > > +static int smmu_sync_ste(struct hyp_arm_smmu_v3_device *smmu, u32 sid)
> > > +{
> > > + struct arm_smmu_cmdq_ent cmd = {
> > > + .opcode = CMDQ_OP_CFGI_STE,
> > > + .cfgi.sid = sid,
> > > + .cfgi.leaf = true,
> > > + };
> > > +
> > > + return smmu_send_cmd(smmu, &cmd);
> > > +}
> > > +
> > I see the page tables are properly configured for ARM_SMMU_FEAT_COHERENCY but no
> > handling for the STE or CMDQ, I believe here we should have something as:
> > if (!(smmu->features & ARM_SMMU_FEAT_COHERENCY))
> > kvm_flush_dcache_to_poc(step, STRTAB_STE_DWORDS << 3);
> >
> > Similarly in "smmu_add_cmd" for the command queue. Or use NC mapping
> > (which doesn't exist
> > upstream as far as I can see)
>
> Right, the host driver seems to do this. If I'm following correctly we end
> up with dma_direct_alloc() calling pgprot_dmacoherent() and get
> MT_NORMAL_NC, when the SMMU is declared non-coherent in DT/IORT.
>
> So we'd get mismatched attributes if hyp is then mapping these structures
> cacheable, but I don't remember how that works exactly. Might be fine
> since host donates the pages to hyp and we'd have a cache flush in
> between. I'll have to read up on that.

I guess that is not enough, as the hypervisor writes the STE/CMDQ at any time.

> Regardless, mapping NC seems cleaner, more readable. I'll see if I can add
> that attribute to kvm_pgtable_hyp_map().

There is a patch for that already in Android
https://android.googlesource.com/kernel/common/+/636c912401dec4d178f6cdf6073f546b15828cf7%5E%21/#F0

But I a guess as beginning CMO is enough, I have this POC for it
https://android-kvm.googlesource.com/linux/+/193b027de376317eb8daa4eb207badaa1d6fda4a%5E%21/#F0

Thanks,
Mostafa