Re: [XEN RFC PATCH 04/40] xen/arm: return default DMA bit width when platform is not set

[Julien Grall <julien@xxxxxxx>]

On 20/08/2021 10:37, Wei Chen wrote:
> Hi Julien,
Hi Wei,

> > -----Original Message-----
> > From: Julien Grall <julien@xxxxxxx>
> > Sent: 2021年8月20日 16:20
> > To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> > sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> > Subject: Re: [XEN RFC PATCH 04/40] xen/arm: return default DMA bit width
> > when platform is not set
> >
> >
> >
> > On 20/08/2021 03:04, Wei Chen wrote:
> > > Hi Julien,
> > Hi Wei,
> >
> > > > -----Original Message-----
> > > > From: Julien Grall <julien@xxxxxxx>
> > > > Sent: 2021年8月19日 21:28
> > > > To: Wei Chen <Wei.Chen@xxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx;
> > > > sstabellini@xxxxxxxxxx; jbeulich@xxxxxxxx
> > > > Cc: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
> > > > Subject: Re: [XEN RFC PATCH 04/40] xen/arm: return default DMA bit
> > width
> > > > when platform is not set
> > > >
> > > > Hi,
> > > >
> > > > On 11/08/2021 11:23, Wei Chen wrote:
> > > > > From: Hongda Deng <Hongda.Deng@xxxxxxx>
> > > > >
> > > > > In current code, arch_get_dma_bitsize will return 32 when platorm
> > > > > or platform->dma_bitsize is not set. It's not resonable, for Arm,
> > > > s/resonable/reasonable/
> > > Ok
> > >
> > > > > we don't require to reserve DMA memory. So we set dma_bitsize always
> > > > > be 0. In NO-NUMA system, arch_get_dma_bitsize will not be invoked,
> > > > > so dma_bitsize will not be overrided by this function.
> > > > arch_get_dma_bitsize() is also used to allocate dom0 memory. We need to
> > > > be able to allocate some DMA-able memory that can be used by every
> > devices.
> > > > > But in NUMA
> > > > > system, once the online nodes are greater than 1, this function will
> > > > > be invoked. The dma_bitsize will be limited to 32. That means, only
> > > > > first 4GB memory can be used for DMA. But that's against our hardware
> > > > > design. We don't have that kind of restriction on hardware.
> > > > What do you mean by "hardware design"? Are you referring to the server
> > > > you boot Xen on?
> > > Yes. I will change it to some neutral words. something like:
> > > "But that could not reflect some hardware's real DMA ability. They may
> > not
> > > have kind of restriction on hardware." ?
> > The thing is DMA ability is not about the platform itself. It is more
> > about the devices (this could just be a PCI card you just plugged). What
> > you seem to suggest is no-one will ever plug such card on your platform.
> > Is that correct?
> OK, I understand now. Let's keep 32-bit as default value, but even in this
> case, how about DMA-16 devices? Although these devices are very rare, they
> still exist : )
I haven't heard anyone reporting issues with them on Xen on Arm. So I 
assume that either it works or no-one is using them.
My main point is we need to care about the common use case. 32-bit DMA 
device is still a thing and caused trouble to some of our users (e.g. NXP).
If tomorrow, someone report issue with 16-bit DMA device, then we can 
consider our options how to handle.
> > > > So I would explore to remove the NUMA check for drop the DMA zone. FAOD,
> > > > both suggestion are for Arm only. For x86, they need to be kept.
> > > Without introducing new flag, such as lowmem_for_dma, it's a little
> > > hard to skip the numa node check. Unless we crudely add #ifdef ARCH to
> > > common code, which is not what we want to see ...
> > >         if ( !dma_bitsize && (num_online_nodes() > 1) )
> > >             dma_bitsize = arch_get_dma_bitsize();
> > ... Why do you think we need this check on Arm when NUMA is enabled?
> I didn't think Arm needs, what I said is introduce a flag to disable
> this check for Arm or other Architectures that they don't need this check.
>
> > We can discuss how to remove it once this is answered.
> I think we can start to discuss it.
How about replacing the second part of the check with a new helper 
arch_have_default_dma_zone() (or a different name)?
Cheers,

--
Julien Grall