Re: [RFC PATCH] xen/arm: arm32: Enable smpboot on Arm32 based systems

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 3 May 2023, Julien Grall wrote:
> Hi Stefano,
> 
> On 03/05/2023 00:55, Stefano Stabellini wrote:
> > > +    {
> > > +        printk("CPU%d: No release addr\n", cpu);
> > > +        return -ENODEV;
> > > +    }
> > > +
> > > +    release = ioremap_nocache(cpu_release_addr[cpu], 4);
> > > +    if ( !release )
> > > +    {
> > > +        dprintk(XENLOG_ERR, "CPU%d: Unable to map release address\n",
> > > cpu);
> > > +        return -EFAULT;
> > > +    }
> > > +
> > > +    writel(__pa(init_secondary), release);
> > > +
> > > +    iounmap(release);
> > 
> > I think we need a wmb() ?
> 
> I am not sure why we would need a wmb() here.

The code does:

writel(__pa(init_secondary), release);
iounmap
sev();

I was thinking of trying to make sure the write is completed before
issuing a sev(). Technically it is possible for the CPU to reorder the
sev() before the write as there is no explicit dependency between the
two?


> Instead, looking at the Linux
> version, I think we are missing a cache flush (so does on arm64) which would
> be necessary if the CPU waiting for the release doesn't have cache enabled.

I thought about it as well but here the patch is calling
ioremap_nocache(), so cache flushes should not be needed?