Re: [PATCH] xen/arm: acpi: Support memory reserve configuration table

[Leo Yan <leo.yan@xxxxxxxxxx>]

On Tue, Sep 06, 2022 at 08:53:02AM +0100, Marc Zyngier wrote:

[...]

> > Okay, I think have two questions for you:
> > 
> > - The first question is if we really need to reserve persistent memory
> >   for RD pending table and configuration table when Linux kernel runs
> >   in Xen domain?
> 
> I have no idea, and really I don't want to know. The architecture
> doesn't make it safe to reuse that memory, and the driver does the
> right thing by always reserving that memory when the FW is supposed to
> support it.
> 
> The "oh but it is safe on so and so" approach doesn't scale. If you
> want to have such a thing, just convince people at ARM that it is
> possible to implement a GICv3-compliant system without the RD tables,
> get them to update the architecture to allow this scheme and advertise
> it in a discoverable register. Xen could then implement it, Linux
> could check this bit, and we'd all be a happy family.

I agree that my patch is not based on a scale approach, this is also
my concern.

To be honest, convincing Arm GIC team is a bit out of my working scope.
I am working on automative project, when I saw verbose log with bunch of
kernel warnings with Xen, it motivated me to chase down, this is the
main reason I tried to explore some solutions at here.

> Because that's really what this is: it isn't that you don't care about
> the RD tables being reserved. It is that you don't care about them at
> all because they are never used by Xen as the GIC implementation. Your
> approach of "huh, let's not reserve it" just papers over this.

> > - If the first question's answer is no, so it's not necessary to reserve
> >   RD pending table and configuration table for Xen, then what's the good
> >   way to dismiss the kernel oops?
> 
> A warning, not an oops.
> 
> > 
> > IIUC, you consider the general flow from architecture view, so you prefer
> > to ask Xen to implement EFI stub to comply the general flow for EFI
> > booting sequence, right?
> 
> If you want to use ACPI, you use EFI, and not a vague emulation of
> it. If you use DT, you can reserve the memory upfront. The various
> alternatives are in this thread.

Given I proposed two fixes, one is for Xen and another is for kernel,
but both are rejected, so I would like to hold on a bit for this.

Maybe it's a good point for Xen maintainers to review if it needs to
support EFI stub, or welcome any other suggestions.

> > If the conclusion is to change Xen for support EFI stub, then this
> > would be fine for me and I will hold on and leave Xen developers to work
> > on it.
> > 
> > > > [1] 
> > > > https://lore.kernel.org/lkml/20220906024040.503764-1-leo.yan@xxxxxxxxxx/T/#u
> > > 
> > > I'm totally baffled by the fact you're trying to add some extra hacks
> > > to Linux just to paper over some of the Xen's own issues.
> > 
> > I have a last question for why kernel reserves RD pending table and
> > configuration table for kexec.  As we know, the primary kernel and
> > the secondary kernel use separate memory regions,
> 
> No, you got it wrong. Only with *kdump* do you get separate memory
> regions. kexec reuses all of the memory visible by the primary kernel.

Thanks for correction.

> > this means there have
> > no race condition that secondary kernel modifies the tables whilist the
> > GIC accesses the table if the secondary kernel allocates new pages for
> > RD tables.  So only one potential issue I can image is the secondary
> > kernel sets new RD pending table and configuration table, which might
> > introduce inconsistent issue with rest RDs in the system.
> > 
> > Could you confirm if my understanding is correct or not?
> 
> It isn't correct.
> 
> - There is no race condition. Once the RD tables are configured, they
>   cannot be changed.
> 
> - When the kdump kernel boots, none of the primary OS memory is
>   reused, so it is safe to continue and use the same tables in place
> 
> - When the kexec kernel boots, all of the memory except for the
>   reserved memory is reused. If your RD tables are used for anything,
>   you'll see memory corruption as the GIC writes pending bits in the
>   pending table, and you'll be unable to configure interrupts
>   correctly.

This gives me impression that when do a power cycle, CPUs are reset
but GIC is still alive, so for every booting time the same memory
region should be reserved for RD tables.

To be honest, it's a bit weird for me that if a system cannot reset
CPUs and GIC together.

> In conclusion, using kexec with GICv3 is completely unsafe if you
> don't reserve the memory allocated to the RDs.
> 
> > Sorry for noise and many questions.  I understand this is a complex
> > and difficult topic for me, and it's very likely that I am absent
> > sufficient knowledge for this part, this is just what I want to
> > learn from the discussion and from you :-)
> 
> I suggest you read the architecture spec, which has all the details.

Thank you for many suggestions!  I read a bit for GICv3/v4 arch spec
at last week, but I need to do more homework.

Thanks,
Leo