Re: [RFC PATCH v4 6/8] xen/arm: scmi: introduce SCI SCMI SMC multi-agent driver

[Julien Grall <julien@xxxxxxx>]

Hi,

On 18/06/2025 00:38, Stefano Stabellini wrote:
> On Thu, 12 Jun 2025, Grygorii Strashko wrote:
> > On 02.06.25 10:17, Bertrand Marquis wrote:
> > > > On the other hand, if we also want to handle the case where the SCMI
> > > > server could be on a separate co-processor, then what this code is doing
> > > > is not sufficient because we also need a dcache flush, in addition to
> > > > the DSB.
> > > >
> > > > Bertrand, can you double-check?
> > > If we want to handle a case where the memory is accessible to a coprocessor
> > > but there is no cache coherency, we need to flush the dcache definitely.
> > >
> > > Seeing the amount of data here, I do agree with Stefano that it would be a
> > > good
> > > idea to make the provision to flush the data cache in all cases. Even if the
> > > data
> > > is accessed by a secure partition or the firmware coherently, flushing in
> > > all cases
> > > would have very limited performance impact here.
> > >
> > > There is the other solution to have some kind of parameter to say if the
> > > accessor
> > > has coherent cache access but I do not think the performance impact here
> > > would
> > > justify such a complexity.
> > The SCMI shmem expected to be mapped as MT_NON_CACHEABLE in all cases.
I can't find MT_NON_CACHEABLE anywhere in Xen or Linux. My 
interpretation is that the memory attribute would be normal memory non 
cacheable. However, this doesn't add up with ...
> > The Linux does devm_ioremap() -> ioremap() ->
> > (ARM64)  __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE))
... this line. This is device nGnRE which is a lot more restrictive (for 
instance it doesn't allow unaligned access).
> > There is also note in docs:
> > "+- shmem: shared memory for messages transfer, **Xen page aligned** with
> > mapping``p2m_mmio_direct_nc``."
> >
> > In the case of SCP - the SCMI shmem can be actually be in SRAM.
> >
> > So, are you sure cache manipulations are required here?
> No, if the memory is mapped as uncacheable everywhere then the cache
> manipulations are not needed. However, we probably still need a dsb.
>
> I understand now why they decided to use __memcpy_fromio in Linux: it is
> not MMIO but they needed a memcpy followed by DSB, so they decided to
> reuse the existing MMIO functions although the buffer is not MMIO.
From my understanding, memcpy_fromio() is not just a mempcy() + dsb. It 
also guarantees the access will be aligned (this is not guarantee by our 
memcpy()).
Now the question is why does Linux map the region Device nGnRE but we 
are mapping non-cacheable?
Cheers,

--
Julien Grall