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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Thu, Sep 28, 2023 at 07:49:18PM -0700, Elliott Mitchell wrote:
> I'm trying to get FreeBSD/ARM operational on Xen/ARM.  Current issue is
> the changes with the handling of the shared information page appear to
> have broken things for me.
> 
> With a pre-4.17 build of Xen/ARM things worked fine.  Yet with a build
> of the 4.17 release, mapping the shared information page doesn't work.

This is due to 71320946d5edf AFAICT.

> I'm using Tianocore as the first stage loader.  This continues to work
> fine.  The build is using tag "edk2-stable202211", commit fff6d81270.
> While Tianocore does map the shared information page, my reading of their
> source is that it properly unmaps the page and therefore shouldn't cause
> trouble.
> 
> Notes on the actual call is gpfn was 0x0000000000040072.  This is outside
> the recommended address range, but my understanding is this is supposed
> to be okay.
> 
> The return code is -16, which is EBUSY.
> 
> Ideas?

I think the issue is that you are mapping the shared info page over a
guest RAM page, and in order to do that you would fist need to create
a hole and then map the shared info page.  IOW: the issue is not with
edk2 not having unmapped the page, but with FreeBSD trying to map the
shared_info over a RAM page instead of a hole in the p2m.  x86
behavior is different here, and does allow mapping the shared_info
page over a RAM gfn (by first removing the backing RAM page on the
gfn).

Ideally we would like to use an unpopulated gfn, but doing so is not
trivial right now, as the point where the shared_info page is mapped
we don't yet have an easy way to account for unpopulated regions.

My suggestion would be to do something like:

diff --git a/sys/x86/xen/hvm.c b/sys/x86/xen/hvm.c
index 4122daeaf600..7251bc69ae15 100644
--- a/sys/x86/xen/hvm.c
+++ b/sys/x86/xen/hvm.c
@@ -194,18 +194,20 @@ xen_hvm_init_shared_info_page(void)
 {
        struct xen_add_to_physmap xatp;
 
-       if (xen_pv_domain()) {
-               /*
-                * Already setup in the PV case, shared_info is passed inside
-                * of the start_info struct at start of day.
-                */
-               return;
-       }
-
        if (HYPERVISOR_shared_info == NULL) {
+               struct xen_remove_from_physmap rm = {
+                       .domid = DOMID_SELF,
+               };
+               int rc;
+
                HYPERVISOR_shared_info = malloc(PAGE_SIZE, M_XENHVM, M_NOWAIT);
                if (HYPERVISOR_shared_info == NULL)
                        panic("Unable to allocate Xen shared info page");
+
+               rm.gpfn = vtophys(HYPERVISOR_shared_info) >> PAGE_SHIFT;
+               rc = HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &rm);
+               if (rc != 0)
+                       printf("Failed to remove shared_info GFN: %d\n", rc);
        }
 
        xatp.domid = DOMID_SELF;

But in the long term we should see about initializing the shared_info
page as part of xenpv_attach().

Regards, Roger.