There is no need to use a populated memory page for mapping the shared
info page at that location. Just use an allocated virtual address for
the shared info page. For PVH allocate an unused pfn.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
arch/x86/mm.c | 7 -------
arch/x86/setup.c | 15 ++++++++-------
arch/x86/x86_32.S | 7 +------
arch/x86/x86_64.S | 7 +------
hypervisor.c | 15 +++++++++++----
5 files changed, 21 insertions(+), 30 deletions(-)
diff --git a/arch/x86/mm.c b/arch/x86/mm.c
index 3f5c7ea7..78d614a6 100644
--- a/arch/x86/mm.c
+++ b/arch/x86/mm.c
@@ -497,7 +497,6 @@ static void build_pagetable(unsigned long *start_pfn, unsigned long *max_pfn)
/*
* Mark portion of the address space read only.
*/
-extern struct shared_info shared_info;
struct change_readonly_par {
unsigned long etext;
@@ -519,12 +518,6 @@ static int change_readonly_func(unsigned long va, unsigned int lvl,
if ( va + (1UL << ptdata[lvl].shift) > ro->etext )
return 1;
- if ( va == (unsigned long)&shared_info )
- {
- printk("skipped %lx\n", va);
- return 0;
- }
-
newval = ro->readonly ? (*pte & ~_PAGE_RW) : (*pte | _PAGE_RW);
#ifdef CONFIG_PARAVIRT
diff --git a/arch/x86/setup.c b/arch/x86/setup.c
index 299ff8c7..8fd55c51 100644
--- a/arch/x86/setup.c
+++ b/arch/x86/setup.c
@@ -47,8 +47,6 @@ shared_info_t *HYPERVISOR_shared_info;
*/
char stack[2*STACK_SIZE];
-extern char shared_info[PAGE_SIZE];
-
static inline void fpu_init(void) {
asm volatile("fninit");
}
@@ -76,18 +74,21 @@ static void set_info_ptr(start_info_t *ptr)
#define hpc_init()
+static unsigned long shared_info_va;
+
shared_info_t *map_shared_info(void)
{
int rc;
- unsigned long pa = start_info_ptr->shared_info;
- if ( (rc = HYPERVISOR_update_va_mapping((unsigned long)shared_info,
- __pte(pa | 7), UVMF_INVLPG)) )
+ if ( !shared_info_va )
+ shared_info_va = alloc_virt_kernel(1);
+ rc = map_frame_rw(shared_info_va, PHYS_PFN(start_info_ptr->shared_info));
+ if ( rc )
{
printk("Failed to map shared_info!! rc=%d\n", rc);
do_exit();
}
- return (shared_info_t *)shared_info;
+ return (shared_info_t *)shared_info_va;
}
void unmap_shared_info(void)
@@ -95,7 +96,7 @@ void unmap_shared_info(void)
int rc;
pte_t nullpte = { };
- if ( (rc = HYPERVISOR_update_va_mapping((unsigned long)shared_info,
+ if ( (rc = HYPERVISOR_update_va_mapping(shared_info_va,
nullpte, UVMF_INVLPG)) )
{
printk("Failed to unmap shared_info page!! rc=%d\n", rc);
diff --git a/arch/x86/x86_32.S b/arch/x86/x86_32.S
index 3de00277..5d891164 100644
--- a/arch/x86/x86_32.S
+++ b/arch/x86/x86_32.S
@@ -36,13 +36,8 @@ _start:
stack_start:
.long stack+(2*__STACK_SIZE), __KERNEL_SS
-.globl shared_info, hypercall_page
- /* Unpleasant -- the PTE that maps this page is actually overwritten */
- /* to map the real shared-info page! :-) */
.align __PAGE_SIZE
-shared_info:
- .fill __PAGE_SIZE,1,0
-
+.globl hypercall_page
hypercall_page:
.fill __PAGE_SIZE,1,0
diff --git a/arch/x86/x86_64.S b/arch/x86/x86_64.S
index 7529c02e..09b93e39 100644
--- a/arch/x86/x86_64.S
+++ b/arch/x86/x86_64.S
@@ -33,13 +33,8 @@ _start:
stack_start:
.quad stack+(2*__STACK_SIZE)
-.globl shared_info, hypercall_page
- /* Unpleasant -- the PTE that maps this page is actually overwritten */
- /* to map the real shared-info page! :-) */
.align __PAGE_SIZE
-shared_info:
- .fill __PAGE_SIZE,1,0
-
+.globl hypercall_page
hypercall_page:
.fill __PAGE_SIZE,1,0
diff --git a/hypervisor.c b/hypervisor.c
index 6476d658..519a4680 100644
--- a/hypervisor.c
+++ b/hypervisor.c
@@ -27,8 +27,10 @@
#include <mini-os/os.h>
#include <mini-os/lib.h>
+#include <mini-os/e820.h>
#include <mini-os/hypervisor.h>
#include <mini-os/events.h>
+#include <mini-os/mm.h>
#include <xen/memory.h>
EXPORT_SYMBOL(hypercall_page);
@@ -37,7 +39,8 @@ EXPORT_SYMBOL(hypercall_page);
((sh)->evtchn_pending[idx] & ~(sh)->evtchn_mask[idx])
#ifndef CONFIG_PARAVIRT
-extern shared_info_t shared_info;
+static unsigned long shinfo_pfn;
+static unsigned long shinfo_va;
int hvm_get_parameter(int idx, uint64_t *value)
{
@@ -69,14 +72,16 @@ shared_info_t *map_shared_info(void)
{
struct xen_add_to_physmap xatp;
+ shinfo_pfn = e820_get_reserved_pfns(1);
xatp.domid = DOMID_SELF;
xatp.idx = 0;
xatp.space = XENMAPSPACE_shared_info;
- xatp.gpfn = virt_to_pfn(&shared_info);
+ xatp.gpfn = shinfo_pfn;
if ( HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
BUG();
+ shinfo_va = map_frame_virt(shinfo_pfn);
- return &shared_info;
+ return (shared_info_t *)shinfo_va;
}
void unmap_shared_info(void)
@@ -84,9 +89,11 @@ void unmap_shared_info(void)
struct xen_remove_from_physmap xrtp;
xrtp.domid = DOMID_SELF;
- xrtp.gpfn = virt_to_pfn(&shared_info);
+ xrtp.gpfn = shinfo_pfn;
if ( HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrtp) != 0 )
BUG();
+ unmap_frames(shinfo_va, 1);
+ e820_put_reserved_pfns(shinfo_pfn, 1);
}
#endif
--
2.43.0On 08.07.2025 08:37, Juergen Gross wrote:
> --- a/hypervisor.c
> +++ b/hypervisor.c
> @@ -27,8 +27,10 @@
>
> #include <mini-os/os.h>
> #include <mini-os/lib.h>
> +#include <mini-os/e820.h>
> #include <mini-os/hypervisor.h>
> #include <mini-os/events.h>
> +#include <mini-os/mm.h>
> #include <xen/memory.h>
>
> EXPORT_SYMBOL(hypercall_page);
> @@ -37,7 +39,8 @@ EXPORT_SYMBOL(hypercall_page);
> ((sh)->evtchn_pending[idx] & ~(sh)->evtchn_mask[idx])
>
> #ifndef CONFIG_PARAVIRT
> -extern shared_info_t shared_info;
> +static unsigned long shinfo_pfn;
> +static unsigned long shinfo_va;
>
> int hvm_get_parameter(int idx, uint64_t *value)
> {
> @@ -69,14 +72,16 @@ shared_info_t *map_shared_info(void)
> {
> struct xen_add_to_physmap xatp;
>
> + shinfo_pfn = e820_get_reserved_pfns(1);
> xatp.domid = DOMID_SELF;
> xatp.idx = 0;
> xatp.space = XENMAPSPACE_shared_info;
> - xatp.gpfn = virt_to_pfn(&shared_info);
> + xatp.gpfn = shinfo_pfn;
> if ( HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
> BUG();
> + shinfo_va = map_frame_virt(shinfo_pfn);
In the PV variant you first check whether you already have a VA. Why is
that needed there, but not here? (Originally I meant to ask why you don't
use map_frame_virt() there as well.)
Talking of map_frame_virt() - I take it its 2nd parameter being named
"mfn" is kind of stale (pre-dating the addition of PVH support)?
> @@ -84,9 +89,11 @@ void unmap_shared_info(void)
> struct xen_remove_from_physmap xrtp;
>
> xrtp.domid = DOMID_SELF;
> - xrtp.gpfn = virt_to_pfn(&shared_info);
> + xrtp.gpfn = shinfo_pfn;
> if ( HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrtp) != 0 )
> BUG();
> + unmap_frames(shinfo_va, 1);
Better do this ahead of the hypercall?
Jan
On 28.07.25 16:21, Jan Beulich wrote:
> On 08.07.2025 08:37, Juergen Gross wrote:
>> --- a/hypervisor.c
>> +++ b/hypervisor.c
>> @@ -27,8 +27,10 @@
>>
>> #include <mini-os/os.h>
>> #include <mini-os/lib.h>
>> +#include <mini-os/e820.h>
>> #include <mini-os/hypervisor.h>
>> #include <mini-os/events.h>
>> +#include <mini-os/mm.h>
>> #include <xen/memory.h>
>>
>> EXPORT_SYMBOL(hypercall_page);
>> @@ -37,7 +39,8 @@ EXPORT_SYMBOL(hypercall_page);
>> ((sh)->evtchn_pending[idx] & ~(sh)->evtchn_mask[idx])
>>
>> #ifndef CONFIG_PARAVIRT
>> -extern shared_info_t shared_info;
>> +static unsigned long shinfo_pfn;
>> +static unsigned long shinfo_va;
>>
>> int hvm_get_parameter(int idx, uint64_t *value)
>> {
>> @@ -69,14 +72,16 @@ shared_info_t *map_shared_info(void)
>> {
>> struct xen_add_to_physmap xatp;
>>
>> + shinfo_pfn = e820_get_reserved_pfns(1);
>> xatp.domid = DOMID_SELF;
>> xatp.idx = 0;
>> xatp.space = XENMAPSPACE_shared_info;
>> - xatp.gpfn = virt_to_pfn(&shared_info);
>> + xatp.gpfn = shinfo_pfn;
>> if ( HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp) != 0 )
>> BUG();
>> + shinfo_va = map_frame_virt(shinfo_pfn);
>
> In the PV variant you first check whether you already have a VA. Why is
> that needed there, but not here? (Originally I meant to ask why you don't
> use map_frame_virt() there as well.)
Good catch. It is needed here, otherwise we'll leak the virtual page
during a suspend/resume cycle.
>
> Talking of map_frame_virt() - I take it its 2nd parameter being named
> "mfn" is kind of stale (pre-dating the addition of PVH support)?
I think it was misnamed back then.
>
>> @@ -84,9 +89,11 @@ void unmap_shared_info(void)
>> struct xen_remove_from_physmap xrtp;
>>
>> xrtp.domid = DOMID_SELF;
>> - xrtp.gpfn = virt_to_pfn(&shared_info);
>> + xrtp.gpfn = shinfo_pfn;
>> if ( HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrtp) != 0 )
>> BUG();
>> + unmap_frames(shinfo_va, 1);
>
> Better do this ahead of the hypercall?
Agreed.
Juergen
© 2016 - 2025 Red Hat, Inc.