Re: Issue with shared information page on Xen/ARM 4.17

[Elliott Mitchell <ehem+xen@xxxxxxx>]

On Wed, Oct 04, 2023 at 03:52:54PM -0700, Stefano Stabellini wrote:
> On Wed, 4 Oct 2023, Julien Grall wrote:
> > > > If we want to handle such firmware, I think it would be better if we
> > > > provide
> > > > an hypercall that would return the GFN where it is currently mapped.
> > > 
> > > Sure, but such hypercall would be racy, as by the time the gfn is
> > > returned the value could be stale.  Adding a replacing and non
> > > replacing XENMEM_add_to_physmap variations would IMO be better.
> > > 
> > > Anyway, I don't maintain this, so it's up to you.
> > 
> > Bertrand/Stefano, any opinions?
> 
> I think we should fix EDK2 to unmap the shared info in all cases as
> that's simpler and the best implementation. What's the value of keeping
> the mapping around when the OS can't find it? Unless you have an idea on
> how the OS could find the location of the existing EDK2 shared info
> mapping.

I tend to agree.

> It is important not to have 2 different behaviors for the same hypercall
> on ARM and x86, especially when the hypercall looks arch-neutral and an
> operating system would reasonably expect to use it in common code.
> Having different behaviors on ARM/x86 is more error prone than having a
> less-than-ideal hypercall implementation.

I attempted to head this direction with FreeBSD, but there were enough
surrounding differences to make this troublesome to implement.  May be
easier on other OSes with less history though.

I do agree on general principle fewer/smaller differences are better.
Yet I note my earlier patch to have `typedef struct trap_info trap_info_t`
consistently visible didn't go through...

> I agree with Julien that the ARM behavior is the right behavior. Can we
> change the x86 implementation to match ARM somehow?
> 
> If we do, I guess we would end up breaking legacy EDK2?

I agree this ARM behavior does seem appropriate.  Due to longer history
though it will need far more transition time.  If this 4.18 tarballs
aren't out yet, I would try to see whether a warning message could be
implemented *now*.  I estimate the old behavior will need support for
5-10 years.


On Wed, Oct 04, 2023 at 03:42:00PM -0700, Stefano Stabellini wrote:
> Sorry to be pedantic but I am really not familiar with EDK2. Does
> "reserved page" in this context mean a memory page from a reserved
> region marked as reserved in the EFI memory map?

I'm not too familiar either, so a fair amount of what I write is
speculation or guesses.  I'm assuming this refers to "ACPI reserved
page", meaning marked as not for use by the OS.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg@xxxxxxx  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445