[PATCH v2] drivers: xen: unmap Enlighten page before jumping to Linux

This commit fixes issue with usage of Xen hypervisor shared info page. Previously U-boot did not unmap it at the end of OS boot process. Xen did not prevent guest from this. So, it worked, but caused wierd issues - one memory page, that was returned by memalign in U-boot for Enlighten mapping was not unmaped by Xen (shared_info values was not removed from there) and returned to allocator. During the Linux boot, it uses shared_info page as regular RAM page, which leads to hypervisor shared info corruption.
So, to fix this issue, as discussed on the xen-devel mailing list, the code should: 1) Unmap the page 2) Populate the area with memory using XENMEM_populate_physmap
This patch adds page unmapping via XENMEM_remove_from_physmap, fills hole in address space where page was mapped via XENMEM_populate_physmap and return this address to memory allocator for freeing.
Signed-off-by: Dmytro Firsov dmytro_firsov@epam.com ---
Changes in v2: - Reword commit message to be more clear with purpose of the patch - Change BUG() helper to panic() and add error messages - Add struct zeroing during initialization - Fix typo in comment --- drivers/xen/hypervisor.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/xen/hypervisor.c b/drivers/xen/hypervisor.c index 2560894832..16c7c96c94 100644 --- a/drivers/xen/hypervisor.c +++ b/drivers/xen/hypervisor.c @@ -144,6 +144,36 @@ struct shared_info *map_shared_info(void *p) return HYPERVISOR_shared_info; }
+void unmap_shared_info(void) +{ + xen_pfn_t shared_info_pfn = virt_to_pfn(HYPERVISOR_shared_info); + struct xen_remove_from_physmap xrfp = {0}; + struct xen_memory_reservation reservation = {0}; + xen_ulong_t nr_exts = 1; + + xrfp.domid = DOMID_SELF; + xrfp.gpfn = shared_info_pfn; + if (HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrfp) != 0) + panic("Failed to unmap HYPERVISOR_shared_info\n"); + + /* + * After removing from physmap there will be a hole in address space on + * HYPERVISOR_shared_info address, so to free memory allocated with + * memalign and prevent exceptions during access to this page we need to + * fill this 4KB hole with XENMEM_populate_physmap before jumping to Linux. + */ + reservation.domid = DOMID_SELF; + reservation.extent_order = 0; + reservation.address_bits = 0; + set_xen_guest_handle(reservation.extent_start, &shared_info_pfn); + reservation.nr_extents = nr_exts; + if (HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation) != nr_exts) + panic("Failed to populate memory on HYPERVISOR_shared_info addr\n"); + + /* Now we can return this to memory allocator */ + free(HYPERVISOR_shared_info); +} + void do_hypervisor_callback(struct pt_regs *regs) { unsigned long l1, l2, l1i, l2i; @@ -251,4 +281,5 @@ void xen_fini(void) fini_gnttab(); fini_xenbus(); fini_events(); + unmap_shared_info(); }

Reviewed-by: Anastasiia Lukianenko vicooodin@gmail.com
On Tue, Jul 19, 2022 at 5:55 PM Dmytro Firsov Dmytro_Firsov@epam.com wrote:
This commit fixes issue with usage of Xen hypervisor shared info page. Previously U-boot did not unmap it at the end of OS boot process. Xen did not prevent guest from this. So, it worked, but caused wierd issues - one memory page, that was returned by memalign in U-boot for Enlighten mapping was not unmaped by Xen (shared_info values was not removed from there) and returned to allocator. During the Linux boot, it uses shared_info page as regular RAM page, which leads to hypervisor shared info corruption.
So, to fix this issue, as discussed on the xen-devel mailing list, the code should:
- Unmap the page
- Populate the area with memory using XENMEM_populate_physmap
This patch adds page unmapping via XENMEM_remove_from_physmap, fills hole in address space where page was mapped via XENMEM_populate_physmap and return this address to memory allocator for freeing.
Signed-off-by: Dmytro Firsov dmytro_firsov@epam.com
Changes in v2:
- Reword commit message to be more clear with purpose of the patch
- Change BUG() helper to panic() and add error messages
- Add struct zeroing during initialization
- Fix typo in comment
drivers/xen/hypervisor.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/xen/hypervisor.c b/drivers/xen/hypervisor.c index 2560894832..16c7c96c94 100644 --- a/drivers/xen/hypervisor.c +++ b/drivers/xen/hypervisor.c @@ -144,6 +144,36 @@ struct shared_info *map_shared_info(void *p) return HYPERVISOR_shared_info; }
+void unmap_shared_info(void) +{
xen_pfn_t shared_info_pfn = virt_to_pfn(HYPERVISOR_shared_info);
struct xen_remove_from_physmap xrfp = {0};
struct xen_memory_reservation reservation = {0};
xen_ulong_t nr_exts = 1;
xrfp.domid = DOMID_SELF;
xrfp.gpfn = shared_info_pfn;
if (HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrfp) != 0)
panic("Failed to unmap HYPERVISOR_shared_info\n");
/*
* After removing from physmap there will be a hole in address
space on
* HYPERVISOR_shared_info address, so to free memory allocated with
* memalign and prevent exceptions during access to this page we
need to
* fill this 4KB hole with XENMEM_populate_physmap before jumping
to Linux.
*/
reservation.domid = DOMID_SELF;
reservation.extent_order = 0;
reservation.address_bits = 0;
set_xen_guest_handle(reservation.extent_start, &shared_info_pfn);
reservation.nr_extents = nr_exts;
if (HYPERVISOR_memory_op(XENMEM_populate_physmap, &reservation) !=
nr_exts)
panic("Failed to populate memory on HYPERVISOR_shared_info
addr\n");
/* Now we can return this to memory allocator */
free(HYPERVISOR_shared_info);
+}
void do_hypervisor_callback(struct pt_regs *regs) { unsigned long l1, l2, l1i, l2i; @@ -251,4 +281,5 @@ void xen_fini(void) fini_gnttab(); fini_xenbus(); fini_events();
unmap_shared_info();
}
2.25.1

On 19.07.22 17:55, Dmytro Firsov wrote:
Hello Dmytro
First of all, thanks for fixing this issue.
Patch looks good, just a nit below.
This commit fixes issue with usage of Xen hypervisor shared info page. Previously U-boot did not unmap it at the end of OS boot process. Xen did not prevent guest from this. So, it worked, but caused wierd issues - one memory page, that was returned by memalign in U-boot for Enlighten mapping was not unmaped by Xen (shared_info values was not removed from there) and returned to allocator. During the Linux boot, it uses shared_info page as regular RAM page, which leads to hypervisor shared info corruption.
So, to fix this issue, as discussed on the xen-devel mailing list, the code should: 1) Unmap the page 2) Populate the area with memory using XENMEM_populate_physmap
This patch adds page unmapping via XENMEM_remove_from_physmap, fills hole in address space where page was mapped via XENMEM_populate_physmap and return this address to memory allocator for freeing.
Signed-off-by: Dmytro Firsov dmytro_firsov@epam.com
Changes in v2:
- Reword commit message to be more clear with purpose of the patch
- Change BUG() helper to panic() and add error messages
- Add struct zeroing during initialization
- Fix typo in comment
drivers/xen/hypervisor.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)
diff --git a/drivers/xen/hypervisor.c b/drivers/xen/hypervisor.c index 2560894832..16c7c96c94 100644 --- a/drivers/xen/hypervisor.c +++ b/drivers/xen/hypervisor.c @@ -144,6 +144,36 @@ struct shared_info *map_shared_info(void *p) return HYPERVISOR_shared_info; }
+void unmap_shared_info(void) +{
- xen_pfn_t shared_info_pfn = virt_to_pfn(HYPERVISOR_shared_info);
- struct xen_remove_from_physmap xrfp = {0};
- struct xen_memory_reservation reservation = {0};
- xen_ulong_t nr_exts = 1;
- xrfp.domid = DOMID_SELF;
- xrfp.gpfn = shared_info_pfn;
- if (HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrfp) != 0)
panic("Failed to unmap HYPERVISOR_shared_info\n");
- /*
* After removing from physmap there will be a hole in address space on
* HYPERVISOR_shared_info address, so to free memory allocated with
* memalign and prevent exceptions during access to this page we need to
* fill this 4KB hole with XENMEM_populate_physmap before jumping to Linux.
*/
- reservation.domid = DOMID_SELF;
- reservation.extent_order = 0;
- reservation.address_bits = 0;
I think the explicit field's zeroing could be dropped now.
[snip]

On 21.07.22 18:11, Oleksandr wrote:
On 19.07.22 17:55, Dmytro Firsov wrote:
Hello Dmytro
First of all, thanks for fixing this issue.
Patch looks good, just a nit below.
Hello Oleksandr,
+ reservation.domid = DOMID_SELF; + reservation.extent_order = 0; + reservation.address_bits = 0;
I think the explicit field's zeroing could be dropped now.
Per my understanding, both of this zeros matters, so I decided to set them explicitly (even with struct zeroing):
- for `extent_order` - zero means "no additional shift for PFN_SHIFT", so we are using 4K pages;
- for `address_bits` - zero means "the user has no addressing restriction".
Best regards,
Dmytro.

On Tue, Jul 19, 2022 at 02:55:28PM +0000, Dmytro Firsov wrote:
This commit fixes issue with usage of Xen hypervisor shared info page. Previously U-boot did not unmap it at the end of OS boot process. Xen did not prevent guest from this. So, it worked, but caused wierd issues - one memory page, that was returned by memalign in U-boot for Enlighten mapping was not unmaped by Xen (shared_info values was not removed from there) and returned to allocator. During the Linux boot, it uses shared_info page as regular RAM page, which leads to hypervisor shared info corruption.
So, to fix this issue, as discussed on the xen-devel mailing list, the code should:
- Unmap the page
- Populate the area with memory using XENMEM_populate_physmap
This patch adds page unmapping via XENMEM_remove_from_physmap, fills hole in address space where page was mapped via XENMEM_populate_physmap and return this address to memory allocator for freeing.
Signed-off-by: Dmytro Firsov dmytro_firsov@epam.com Reviewed-by: Anastasiia Lukianenko vicooodin@gmail.com
Applied to u-boot/master, thanks!
participants (4)
-
Dmytro Firsov
-
Nastya Vicodin
-
Oleksandr
-
Tom Rini