* [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs
@ 2013-05-02 16:24 Olaf Hering
2013-05-03 7:12 ` Jan Beulich
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Olaf Hering @ 2013-05-02 16:24 UTC (permalink / raw)
To: xen-devel
# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1367511750 -7200
# Node ID 19777301f89653c640de73c6319c9d386a28327f
# Parent 9df019eef776d129c2abb130d1458914fe1ecac4
xen: handle paged gfn in wrmsr_hypervisor_regs
If xenpaging is started very early for a guest the gfn for the hypercall
page may be paged-out already. This leads to a guest crash:
...
(XEN) HVM10: Allocated Xen hypercall page at 169ff000
(XEN) traps.c:654:d10 Bad GMFN 169ff (MFN 3e900000000) to MSR 40000000
(XEN) HVM10: Detected Xen v4.3
(XEN) io.c:201:d10 MMIO emulation failed @ 0008:c2c2c2c2: 18 7c 55 6d 03 83 ff ff 10 7c
(XEN) hvm.c:1253:d10 Triple fault on VCPU0 - invoking HVM shutdown action 1.
(XEN) HVM11: HVM Loader
...
Handle paged gfns in wrmsr_hypervisor_regs and update callers to deal with the
new return code -EAGAIN.
Also update the gdprintk to handle a page value of NULL to avoid printing a
bogus MFN value.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
diff -r 9df019eef776 -r 19777301f896 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1569,7 +1569,7 @@ static int svm_msr_read_intercept(unsign
static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
- int ret;
+ int ret, retry = 0;
struct vcpu *v = current;
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
int sync = 0;
@@ -1682,14 +1682,15 @@ static int svm_msr_write_intercept(unsig
if ( wrmsr_viridian_regs(msr, msr_content) )
break;
- wrmsr_hypervisor_regs(msr, msr_content);
+ ret = wrmsr_hypervisor_regs(msr, msr_content);
+ retry = ret == -EAGAIN;
break;
}
if ( sync )
svm_vmload(vmcb);
- return X86EMUL_OKAY;
+ return retry ? X86EMUL_RETRY : X86EMUL_OKAY;
gpf:
hvm_inject_hw_exception(TRAP_gp_fault, 0);
diff -r 9df019eef776 -r 19777301f896 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2020,6 +2020,7 @@ void vmx_vlapic_msr_changed(struct vcpu
static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
+ int ret = 0;
struct vcpu *v = current;
HVM_DBG_LOG(DBG_LEVEL_1, "ecx=%#x, msr_value=%#"PRIx64, msr, msr_content);
@@ -2088,7 +2089,9 @@ static int vmx_msr_write_intercept(unsig
case HNDL_unhandled:
if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
!is_last_branch_msr(msr) )
- wrmsr_hypervisor_regs(msr, msr_content);
+ ret = wrmsr_hypervisor_regs(msr, msr_content);
+ if ( ret == -EAGAIN )
+ return X86EMUL_RETRY;
break;
case HNDL_exception_raised:
return X86EMUL_EXCEPTION;
diff -r 9df019eef776 -r 19777301f896 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -634,6 +634,7 @@ int wrmsr_hypervisor_regs(uint32_t idx,
unsigned long gmfn = val >> 12;
unsigned int idx = val & 0xfff;
struct page_info *page;
+ p2m_type_t t;
if ( idx > 0 )
{
@@ -643,15 +644,21 @@ int wrmsr_hypervisor_regs(uint32_t idx,
return 0;
}
- page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+ page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
if ( !page || !get_page_type(page, PGT_writable_page) )
{
if ( page )
put_page(page);
+ if ( p2m_is_paging(t) )
+ {
+ p2m_mem_paging_populate(d, gmfn);
+ return -EAGAIN;
+ }
+
gdprintk(XENLOG_WARNING,
"Bad GMFN %lx (MFN %lx) to MSR %08x\n",
- gmfn, page_to_mfn(page), base + idx);
+ gmfn, page ? page_to_mfn(page) : -1UL, base + idx);
return 0;
}
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-02 16:24 [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs Olaf Hering
@ 2013-05-03 7:12 ` Jan Beulich
2013-05-03 12:58 ` Olaf Hering
2013-05-03 12:57 ` [PATCH v2] " Olaf Hering
` (3 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2013-05-03 7:12 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
>>> On 02.05.13 at 18:24, Olaf Hering <olaf@aepfle.de> wrote:
> @@ -1682,14 +1682,15 @@ static int svm_msr_write_intercept(unsig
> if ( wrmsr_viridian_regs(msr, msr_content) )
> break;
>
> - wrmsr_hypervisor_regs(msr, msr_content);
> + ret = wrmsr_hypervisor_regs(msr, msr_content);
> + retry = ret == -EAGAIN;
If you add error handling, don't constrain this to a single error code
please. For the case here, the easiest would appear to be a switch
converting to X86EMUL_OKAY, X86EMUL_RETRY, or
X86EMUL_UNHANDLEABLE. If the function had ways to fail before,
it would have been a bug anyway to not check the return value.
> @@ -2088,7 +2089,9 @@ static int vmx_msr_write_intercept(unsig
> case HNDL_unhandled:
> if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
> !is_last_branch_msr(msr) )
> - wrmsr_hypervisor_regs(msr, msr_content);
> + ret = wrmsr_hypervisor_regs(msr, msr_content);
> + if ( ret == -EAGAIN )
> + return X86EMUL_RETRY;
Similarly here, obviously.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-02 16:24 [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs Olaf Hering
2013-05-03 7:12 ` Jan Beulich
@ 2013-05-03 12:57 ` Olaf Hering
2013-05-03 13:58 ` Jan Beulich
2013-05-03 15:17 ` [PATCH v3] " Olaf Hering
` (2 subsequent siblings)
4 siblings, 1 reply; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 12:57 UTC (permalink / raw)
To: xen-devel
# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1367585741 -7200
# Node ID 219e97e92394eb8aea443144ae93e48d9cce9d87
# Parent 9df019eef776d129c2abb130d1458914fe1ecac4
xen: handle paged gfn in wrmsr_hypervisor_regs
If xenpaging is started very early for a guest the gfn for the hypercall
page may be paged-out already. This leads to a guest crash:
...
(XEN) HVM10: Allocated Xen hypercall page at 169ff000
(XEN) traps.c:654:d10 Bad GMFN 169ff (MFN 3e900000000) to MSR 40000000
(XEN) HVM10: Detected Xen v4.3
(XEN) io.c:201:d10 MMIO emulation failed @ 0008:c2c2c2c2: 18 7c 55 6d 03 83 ff ff 10 7c
(XEN) hvm.c:1253:d10 Triple fault on VCPU0 - invoking HVM shutdown action 1.
(XEN) HVM11: HVM Loader
...
Handle paged gfns in wrmsr_hypervisor_regs and update callers to deal with the
new return code -EAGAIN.
Also update the gdprintk to handle a page value of NULL to avoid printing a
bogus MFN value.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
diff -r 9df019eef776 -r 219e97e92394 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1569,7 +1569,7 @@ static int svm_msr_read_intercept(unsign
static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
- int ret;
+ int ret, result = X86EMUL_OKAY;
struct vcpu *v = current;
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
int sync = 0;
@@ -1682,14 +1682,25 @@ static int svm_msr_write_intercept(unsig
if ( wrmsr_viridian_regs(msr, msr_content) )
break;
- wrmsr_hypervisor_regs(msr, msr_content);
+ ret = wrmsr_hypervisor_regs(msr, msr_content);
+ switch ( ret )
+ {
+ case -EAGAIN:
+ result = X86EMUL_RETRY;
+ break;
+ case 0:
+ result = X86EMUL_UNHANDLEABLE;
+ break;
+ default:
+ break;
+ }
break;
}
if ( sync )
svm_vmload(vmcb);
- return X86EMUL_OKAY;
+ return result;
gpf:
hvm_inject_hw_exception(TRAP_gp_fault, 0);
diff -r 9df019eef776 -r 219e97e92394 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2020,6 +2020,7 @@ void vmx_vlapic_msr_changed(struct vcpu
static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
+ int ret = 0;
struct vcpu *v = current;
HVM_DBG_LOG(DBG_LEVEL_1, "ecx=%#x, msr_value=%#"PRIx64, msr, msr_content);
@@ -2088,7 +2089,16 @@ static int vmx_msr_write_intercept(unsig
case HNDL_unhandled:
if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
!is_last_branch_msr(msr) )
- wrmsr_hypervisor_regs(msr, msr_content);
+ ret = wrmsr_hypervisor_regs(msr, msr_content);
+ switch ( ret )
+ {
+ case -EAGAIN:
+ return X86EMUL_RETRY;
+ case 0:
+ return X86EMUL_UNHANDLEABLE;
+ default:
+ break;
+ }
break;
case HNDL_exception_raised:
return X86EMUL_EXCEPTION;
diff -r 9df019eef776 -r 219e97e92394 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -634,6 +634,7 @@ int wrmsr_hypervisor_regs(uint32_t idx,
unsigned long gmfn = val >> 12;
unsigned int idx = val & 0xfff;
struct page_info *page;
+ p2m_type_t t;
if ( idx > 0 )
{
@@ -643,15 +644,22 @@ int wrmsr_hypervisor_regs(uint32_t idx,
return 0;
}
- page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+ page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
if ( !page || !get_page_type(page, PGT_writable_page) )
{
if ( page )
put_page(page);
+
+ if ( p2m_is_paging(t) )
+ {
+ p2m_mem_paging_populate(d, gmfn);
+ return -EAGAIN;
+ }
+
gdprintk(XENLOG_WARNING,
"Bad GMFN %lx (MFN %lx) to MSR %08x\n",
- gmfn, page_to_mfn(page), base + idx);
+ gmfn, page ? page_to_mfn(page) : -1UL, base + idx);
return 0;
}
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 7:12 ` Jan Beulich
@ 2013-05-03 12:58 ` Olaf Hering
2013-05-03 13:40 ` Olaf Hering
0 siblings, 1 reply; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 12:58 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Fri, May 03, Jan Beulich wrote:
> >>> On 02.05.13 at 18:24, Olaf Hering <olaf@aepfle.de> wrote:
> > @@ -1682,14 +1682,15 @@ static int svm_msr_write_intercept(unsig
> > if ( wrmsr_viridian_regs(msr, msr_content) )
> > break;
> >
> > - wrmsr_hypervisor_regs(msr, msr_content);
> > + ret = wrmsr_hypervisor_regs(msr, msr_content);
> > + retry = ret == -EAGAIN;
>
> If you add error handling, don't constrain this to a single error code
> please. For the case here, the easiest would appear to be a switch
> converting to X86EMUL_OKAY, X86EMUL_RETRY, or
> X86EMUL_UNHANDLEABLE. If the function had ways to fail before,
> it would have been a bug anyway to not check the return value.
I just sent v2 of this patch.
Olaf
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 12:58 ` Olaf Hering
@ 2013-05-03 13:40 ` Olaf Hering
2013-05-03 13:53 ` Jan Beulich
0 siblings, 1 reply; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 13:40 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Fri, May 03, Olaf Hering wrote:
> On Fri, May 03, Jan Beulich wrote:
>
> > >>> On 02.05.13 at 18:24, Olaf Hering <olaf@aepfle.de> wrote:
> > > @@ -1682,14 +1682,15 @@ static int svm_msr_write_intercept(unsig
> > > if ( wrmsr_viridian_regs(msr, msr_content) )
> > > break;
> > >
> > > - wrmsr_hypervisor_regs(msr, msr_content);
> > > + ret = wrmsr_hypervisor_regs(msr, msr_content);
> > > + retry = ret == -EAGAIN;
> >
> > If you add error handling, don't constrain this to a single error code
> > please. For the case here, the easiest would appear to be a switch
> > converting to X86EMUL_OKAY, X86EMUL_RETRY, or
> > X86EMUL_UNHANDLEABLE. If the function had ways to fail before,
> > it would have been a bug anyway to not check the return value.
>
> I just sent v2 of this patch.
My change v2 causes a boot failure, the return value 0 is not handled
correctly.
Did you really mean to translate ret == 0 to X86EMUL_UNHANDLEABLE?
Olaf
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2020,6 +2020,7 @@ void vmx_vlapic_msr_changed(struct vcpu
static int vmx_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
+ int ret = 0;
struct vcpu *v = current;
HVM_DBG_LOG(DBG_LEVEL_1, "ecx=%#x, msr_value=%#"PRIx64, msr, msr_content);
@@ -2088,7 +2089,17 @@ static int vmx_msr_write_intercept(unsig
case HNDL_unhandled:
if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
!is_last_branch_msr(msr) )
- wrmsr_hypervisor_regs(msr, msr_content);
+ ret = wrmsr_hypervisor_regs(msr, msr_content);
+ printk("%s(%u) msr %x mc %lx r %d\n", __func__, __LINE__, msr, msr_content, ret);
+ switch ( ret )
+ {
+ case -EAGAIN:
+ return X86EMUL_RETRY;
+ case 0:
+ return X86EMUL_UNHANDLEABLE;
+ default:
+ break;
+ }
break;
case HNDL_exception_raised:
return X86EMUL_EXCEPTION;
satriani login: (XEN) HVM2: HVM Loader
(XEN) vmx_msr_write_intercept(2093) msr 40000000 mc 80000 r 1
(XEN) HVM2: Detected Xen v4.3.26961-20130503
(XEN) HVM2: Xenbus rings @0xfeffc000, event channel 6
(XEN) HVM2: System requested SeaBIOS
(XEN) HVM2: CPU speed is 2926 MHz
(XEN) irq.c:270: Dom2 PCI link 0 changed 0 -> 5
(XEN) HVM2: PCI-ISA link 0 routed to IRQ5
(XEN) irq.c:270: Dom2 PCI link 1 changed 0 -> 10
(XEN) HVM2: PCI-ISA link 1 routed to IRQ10
(XEN) irq.c:270: Dom2 PCI link 2 changed 0 -> 11
(XEN) HVM2: PCI-ISA link 2 routed to IRQ11
(XEN) irq.c:270: Dom2 PCI link 3 changed 0 -> 5
(XEN) HVM2: PCI-ISA link 3 routed to IRQ5
(XEN) HVM2: pci dev 01:2 INTD->IRQ5
(XEN) HVM2: pci dev 01:3 INTA->IRQ10
(XEN) HVM2: pci dev 03:0 INTA->IRQ5
(XEN) HVM2: pci dev 02:0 bar 10 size lx: 02000000
(XEN) HVM2: pci dev 03:0 bar 14 size lx: 01000000
(XEN) HVM2: pci dev 02:0 bar 30 size lx: 00010000
(XEN) HVM2: pci dev 02:0 bar 14 size lx: 00001000
(XEN) HVM2: pci dev 03:0 bar 10 size lx: 00000100
(XEN) HVM2: pci dev 01:2 bar 20 size lx: 00000020
(XEN) HVM2: pci dev 01:1 bar 20 size lx: 00000010
(XEN) HVM2: Multiprocessor initialisation:
(XEN) HVM2: - CPU0 ... 40-bit phys ... fixed MTRRs ... var MTRRs [2/8] ... done.
(XEN) HVM2: - CPU1 ... 40-bit phys ... fixed MTRRs ... var MTRRs [2/8] ... done.
(XEN) HVM2: - CPU2 ... 40-bit phys ... fixed MTRRs ... var MTRRs [2/8] ... done.
(XEN) HVM2: - CPU3 ... 40-bit phys ... fixed MTRRs ... var MTRRs [2/8] ... done.
(XEN) HVM2: Testing HVM environment:
(XEN) HVM2: - REP INSB across page boundaries ... passed
(XEN) HVM2: - GS base MSRs and SWAPGS ... passed
(XEN) HVM2: Passed 2 of 2 tests
(XEN) HVM2: Writing SMBIOS tables ...
(XEN) HVM2: Loading SeaBIOS ...
(XEN) HVM2: Creating MP tables ...
(XEN) HVM2: Loading ACPI ...
(XEN) HVM2: vm86 TSS at fc00a100
(XEN) HVM2: BIOS map:
(XEN) HVM2: 10000-100d3: Scratch space
(XEN) HVM2: e0000-fffff: Main BIOS
(XEN) HVM2: E820 table:
(XEN) HVM2: [00]: 00000000:00000000 - 00000000:000a0000: RAM
(XEN) HVM2: HOLE: 00000000:000a0000 - 00000000:000e0000
(XEN) HVM2: [01]: 00000000:000e0000 - 00000000:00100000: RESERVED
(XEN) HVM2: [02]: 00000000:00100000 - 00000000:16a00000: RAM
(XEN) HVM2: HOLE: 00000000:16a00000 - 00000000:fc000000
(XEN) HVM2: [03]: 00000000:fc000000 - 00000001:00000000: RESERVED
(XEN) HVM2: Invoking SeaBIOS ...
(XEN) HVM2: SeaBIOS (version ?-20130503_132523-bax)
(XEN) HVM2:
(XEN) HVM2: Found Xen hypervisor signature at 40000000
(XEN) HVM2: xen: copy e820...
(XEN) HVM2: Ram Size=0x16a00000 (0x0000000000000000 high)
(XEN) HVM2: Relocating low data from 0x000e2490 to 0x000ef790 (size 2156)
(XEN) HVM2: Relocating init from 0x000e2cfc to 0x169e20f0 (size 56804)
(XEN) HVM2: CPU Mhz=2926
(XEN) HVM2: Found 7 PCI devices (max PCI bus is 00)
(XEN) HVM2: Allocated Xen hypercall page at 169ff000
(XEN) vmx_msr_write_intercept(2093) msr 40000000 mc 169ff000 r 1
(XEN) HVM2: Detected Xen v4.3.26961-20130503
(XEN) HVM2: Found 4 cpu(s) max supported 4 cpu(s)
(XEN) HVM2: xen: copy BIOS tables...
(XEN) HVM2: Copying SMBIOS entry point from 0x00010010 to 0x000fdb10
(XEN) HVM2: Copying MPTABLE from 0xfc0011c0/fc0011d0 to 0x000fd9f0
(XEN) HVM2: Copying PIR from 0x00010030 to 0x000fd970
(XEN) HVM2: Copying ACPI RSDP from 0x000100b0 to 0x000fd940
(XEN) HVM2: Scan for VGA option rom
(XEN) HVM2: Running option rom at c000:0003
(XEN) stdvga.c:147:d2 entering stdvga and caching modes
(XEN) HVM2: Turning on vga text mode console
(XEN) HVM2: SeaBIOS (version ?-20130503_132523-bax)
(XEN) HVM2:
(XEN) HVM2: UHCI init on dev 00:01.2 (io=c100)
(XEN) HVM2: Found 1 lpt ports
(XEN) HVM2: Found 1 serial ports
(XEN) HVM2: ATA controller 1 at 1f0/3f4/c120 (irq 14 dev 9)
(XEN) HVM2: ATA controller 2 at 170/374/c128 (irq 15 dev 9)
(XEN) HVM2: ata0-0: QEMU HARDDISK ATA-7 Hard-Disk (10240 MiBytes)
(XEN) HVM2: Searching bootorder for: /pci@i0cf8/*@1,1/drive@0/disk@0
(XEN) HVM2: DVD/CD [ata1-0: QEMU DVD-ROM ATAPI-4 DVD/CD]
(XEN) HVM2: Searching bootorder for: /pci@i0cf8/*@1,1/drive@1/disk@0
(XEN) HVM2: PS2 keyboard initialized
(XEN) HVM2: All threads complete.
(XEN) HVM2: Scan for option roms
(XEN) HVM2: Press F12 for boot menu.
(XEN) HVM2:
(XEN) HVM2: drive 0x000fd8f0: PCHS=16383/16/63 translation=lba LCHS=1024/255/63 s=20971520
(XEN) HVM2:
(XEN) HVM2: Space available for UMB: 000c9000-000ee800
(XEN) HVM2: Returned 61440 bytes of ZoneHigh
(XEN) HVM2: e820 map has 6 items:
(XEN) HVM2: 0: 0000000000000000 - 000000000009fc00 = 1 RAM
(XEN) HVM2: 1: 000000000009fc00 - 00000000000a0000 = 2 RESERVED
(XEN) HVM2: 2: 00000000000f0000 - 0000000000100000 = 2 RESERVED
(XEN) HVM2: 3: 0000000000100000 - 00000000169ff000 = 1 RAM
(XEN) HVM2: 4: 00000000169ff000 - 0000000016a00000 = 2 RESERVED
(XEN) HVM2: 5: 00000000fc000000 - 0000000100000000 = 2 RESERVED
(XEN) HVM2: enter handle_19:
(XEN) HVM2: NULL
(XEN) HVM2: Booting from DVD/CD...
(XEN) HVM2: Booting from 0000:7c00
(XEN) stdvga.c:151:d2 leaving stdvga
(XEN) stdvga.c:147:d2 entering stdvga and caching modes
(XEN) stdvga.c:151:d2 leaving stdvga
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
(XEN) vmx_msr_write_intercept(2093) msr 8b mc 0 r 0
...
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 13:40 ` Olaf Hering
@ 2013-05-03 13:53 ` Jan Beulich
0 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2013-05-03 13:53 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
>>> On 03.05.13 at 15:40, Olaf Hering <olaf@aepfle.de> wrote:
> On Fri, May 03, Olaf Hering wrote:
>
>> On Fri, May 03, Jan Beulich wrote:
>>
>> > >>> On 02.05.13 at 18:24, Olaf Hering <olaf@aepfle.de> wrote:
>> > > @@ -1682,14 +1682,15 @@ static int svm_msr_write_intercept(unsig
>> > > if ( wrmsr_viridian_regs(msr, msr_content) )
>> > > break;
>> > >
>> > > - wrmsr_hypervisor_regs(msr, msr_content);
>> > > + ret = wrmsr_hypervisor_regs(msr, msr_content);
>> > > + retry = ret == -EAGAIN;
>> >
>> > If you add error handling, don't constrain this to a single error code
>> > please. For the case here, the easiest would appear to be a switch
>> > converting to X86EMUL_OKAY, X86EMUL_RETRY, or
>> > X86EMUL_UNHANDLEABLE. If the function had ways to fail before,
>> > it would have been a bug anyway to not check the return value.
>>
>> I just sent v2 of this patch.
>
> My change v2 causes a boot failure, the return value 0 is not handled
> correctly.
> Did you really mean to translate ret == 0 to X86EMUL_UNHANDLEABLE?
Of course I didn't - I merely enumerated the values that I'd expect
to result, with no meaning implied from their ordering.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 12:57 ` [PATCH v2] " Olaf Hering
@ 2013-05-03 13:58 ` Jan Beulich
2013-05-03 14:11 ` Olaf Hering
2013-05-03 14:26 ` Keir Fraser
0 siblings, 2 replies; 27+ messages in thread
From: Jan Beulich @ 2013-05-03 13:58 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
>>> On 03.05.13 at 14:57, Olaf Hering <olaf@aepfle.de> wrote:
> @@ -1682,14 +1682,25 @@ static int svm_msr_write_intercept(unsig
> if ( wrmsr_viridian_regs(msr, msr_content) )
> break;
>
> - wrmsr_hypervisor_regs(msr, msr_content);
> + ret = wrmsr_hypervisor_regs(msr, msr_content);
> + switch ( ret )
> + {
> + case -EAGAIN:
> + result = X86EMUL_RETRY;
> + break;
> + case 0:
> + result = X86EMUL_UNHANDLEABLE;
> + break;
> + default:
> + break;
As you had already noticed the hard way - case 0 and default of
course need to be switched (0 -> okay, anything else ->
unhandleable).
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 13:58 ` Jan Beulich
@ 2013-05-03 14:11 ` Olaf Hering
2013-05-03 14:24 ` Jan Beulich
2013-05-03 14:26 ` Keir Fraser
1 sibling, 1 reply; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 14:11 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Fri, May 03, Jan Beulich wrote:
> >>> On 03.05.13 at 14:57, Olaf Hering <olaf@aepfle.de> wrote:
> > @@ -1682,14 +1682,25 @@ static int svm_msr_write_intercept(unsig
> > if ( wrmsr_viridian_regs(msr, msr_content) )
> > break;
> >
> > - wrmsr_hypervisor_regs(msr, msr_content);
> > + ret = wrmsr_hypervisor_regs(msr, msr_content);
> > + switch ( ret )
> > + {
> > + case -EAGAIN:
> > + result = X86EMUL_RETRY;
> > + break;
> > + case 0:
> > + result = X86EMUL_UNHANDLEABLE;
> > + break;
> > + default:
> > + break;
>
> As you had already noticed the hard way - case 0 and default of
> course need to be switched (0 -> okay, anything else ->
> unhandleable).
I dont follow.
ret == 1 looks like success to me, ret == 0 some sort of failure.
Olaf
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 14:11 ` Olaf Hering
@ 2013-05-03 14:24 ` Jan Beulich
0 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2013-05-03 14:24 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
>>> On 03.05.13 at 16:11, Olaf Hering <olaf@aepfle.de> wrote:
> On Fri, May 03, Jan Beulich wrote:
>
>> >>> On 03.05.13 at 14:57, Olaf Hering <olaf@aepfle.de> wrote:
>> > @@ -1682,14 +1682,25 @@ static int svm_msr_write_intercept(unsig
>> > if ( wrmsr_viridian_regs(msr, msr_content) )
>> > break;
>> >
>> > - wrmsr_hypervisor_regs(msr, msr_content);
>> > + ret = wrmsr_hypervisor_regs(msr, msr_content);
>> > + switch ( ret )
>> > + {
>> > + case -EAGAIN:
>> > + result = X86EMUL_RETRY;
>> > + break;
>> > + case 0:
>> > + result = X86EMUL_UNHANDLEABLE;
>> > + break;
>> > + default:
>> > + break;
>>
>> As you had already noticed the hard way - case 0 and default of
>> course need to be switched (0 -> okay, anything else ->
>> unhandleable).
>
> I dont follow.
> ret == 1 looks like success to me, ret == 0 some sort of failure.
Let me check again... Positive values (or 1 in particular) mean
"handled", 0 means not handled, negative values are errors.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 13:58 ` Jan Beulich
2013-05-03 14:11 ` Olaf Hering
@ 2013-05-03 14:26 ` Keir Fraser
2013-05-03 14:31 ` Jan Beulich
1 sibling, 1 reply; 27+ messages in thread
From: Keir Fraser @ 2013-05-03 14:26 UTC (permalink / raw)
To: Jan Beulich, Olaf Hering; +Cc: xen-devel
On 03/05/2013 14:58, "Jan Beulich" <JBeulich@suse.com> wrote:
>>>> On 03.05.13 at 14:57, Olaf Hering <olaf@aepfle.de> wrote:
>> @@ -1682,14 +1682,25 @@ static int svm_msr_write_intercept(unsig
>> if ( wrmsr_viridian_regs(msr, msr_content) )
>> break;
>>
>> - wrmsr_hypervisor_regs(msr, msr_content);
>> + ret = wrmsr_hypervisor_regs(msr, msr_content);
>> + switch ( ret )
>> + {
>> + case -EAGAIN:
>> + result = X86EMUL_RETRY;
>> + break;
>> + case 0:
>> + result = X86EMUL_UNHANDLEABLE;
>> + break;
>> + default:
>> + break;
>
> As you had already noticed the hard way - case 0 and default of
> course need to be switched (0 -> okay, anything else ->
> unhandleable).
No!
Actually anything other than -EAGAIN should be handled here as 'okay'. In
fact the return codes from wrmsr_hypervisor_regs are going to be a bit of a
mess if we're not careful.
I suggest the following return codes:
0: not handled
1: handled
-EINVAL: error during handling
-EAGAIN: retry
The HVM callers should then handle as follows:
-EAGAIN: rc = X86EMUL_RETRY
-EINVAL: goto gp_fault
0: try other msr handlers (if any)
1: we're done, return X86EMUL_OKAY
Does that make sense?
-- Keir
> Jan
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 14:26 ` Keir Fraser
@ 2013-05-03 14:31 ` Jan Beulich
2013-05-03 15:19 ` Olaf Hering
0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2013-05-03 14:31 UTC (permalink / raw)
To: Olaf Hering, Keir Fraser; +Cc: xen-devel
>>> On 03.05.13 at 16:26, Keir Fraser <keir.xen@gmail.com> wrote:
> On 03/05/2013 14:58, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>>>>> On 03.05.13 at 14:57, Olaf Hering <olaf@aepfle.de> wrote:
>>> @@ -1682,14 +1682,25 @@ static int svm_msr_write_intercept(unsig
>>> if ( wrmsr_viridian_regs(msr, msr_content) )
>>> break;
>>>
>>> - wrmsr_hypervisor_regs(msr, msr_content);
>>> + ret = wrmsr_hypervisor_regs(msr, msr_content);
>>> + switch ( ret )
>>> + {
>>> + case -EAGAIN:
>>> + result = X86EMUL_RETRY;
>>> + break;
>>> + case 0:
>>> + result = X86EMUL_UNHANDLEABLE;
>>> + break;
>>> + default:
>>> + break;
>>
>> As you had already noticed the hard way - case 0 and default of
>> course need to be switched (0 -> okay, anything else ->
>> unhandleable).
>
> No!
>
> Actually anything other than -EAGAIN should be handled here as 'okay'. In
> fact the return codes from wrmsr_hypervisor_regs are going to be a bit of a
> mess if we're not careful.
>
> I suggest the following return codes:
> 0: not handled
> 1: handled
> -EINVAL: error during handling
> -EAGAIN: retry
>
> The HVM callers should then handle as follows:
> -EAGAIN: rc = X86EMUL_RETRY
> -EINVAL: goto gp_fault
> 0: try other msr handlers (if any)
> 1: we're done, return X86EMUL_OKAY
>
> Does that make sense?
Sure - you may have seen my later reply where I also notide
this mistake in my first response to Olaf.
The only thing I'd like to ensure is to not constrain the code to
specific error codes - any negative value other than -EAGAIN
should result in #GP (or whatever a suitable action is) imo.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-02 16:24 [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs Olaf Hering
2013-05-03 7:12 ` Jan Beulich
2013-05-03 12:57 ` [PATCH v2] " Olaf Hering
@ 2013-05-03 15:17 ` Olaf Hering
2013-05-03 15:30 ` Jan Beulich
2013-05-03 15:55 ` Keir Fraser
2013-05-03 15:43 ` [PATCH v4] " Olaf Hering
2013-05-03 16:29 ` [PATCH v5] " Olaf Hering
4 siblings, 2 replies; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 15:17 UTC (permalink / raw)
To: xen-devel
# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1367593457 -7200
# Node ID b8af60cf8282bfddb13cc10e4ffaf0c396a15104
# Parent 9df019eef776d129c2abb130d1458914fe1ecac4
xen: handle paged gfn in wrmsr_hypervisor_regs
If xenpaging is started very early for a guest the gfn for the hypercall
page may be paged-out already. This leads to a guest crash:
...
(XEN) HVM10: Allocated Xen hypercall page at 169ff000
(XEN) traps.c:654:d10 Bad GMFN 169ff (MFN 3e900000000) to MSR 40000000
(XEN) HVM10: Detected Xen v4.3
(XEN) io.c:201:d10 MMIO emulation failed @ 0008:c2c2c2c2: 18 7c 55 6d 03 83 ff ff 10 7c
(XEN) hvm.c:1253:d10 Triple fault on VCPU0 - invoking HVM shutdown action 1.
(XEN) HVM11: HVM Loader
...
Update return codes of wrmsr_hypervisor_regs, update callers to deal
with the new return codes:
0: not handled
1: handled
-EINVAL: error during handling
-EAGAIN: retry
Also update the gdprintk to handle a page value of NULL to avoid
printing a bogus MFN value. Update also computing of MSR value in
gdprintk, the idx was always zero.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1569,7 +1569,7 @@ static int svm_msr_read_intercept(unsign
static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
- int ret;
+ int ret, result = X86EMUL_OKAY;
struct vcpu *v = current;
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
int sync = 0;
@@ -1682,14 +1682,24 @@ static int svm_msr_write_intercept(unsig
if ( wrmsr_viridian_regs(msr, msr_content) )
break;
- wrmsr_hypervisor_regs(msr, msr_content);
+ switch ( wrmsr_hypervisor_regs(msr, msr_content) )
+ {
+ case -EAGAIN:
+ result = X86EMUL_RETRY;
+ break;
+ case 0:
+ case 1:
+ break;
+ default:
+ goto gpf;
+ }
break;
}
if ( sync )
svm_vmload(vmcb);
- return X86EMUL_OKAY;
+ return result;
gpf:
hvm_inject_hw_exception(TRAP_gp_fault, 0);
diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2088,7 +2088,16 @@ static int vmx_msr_write_intercept(unsig
case HNDL_unhandled:
if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
!is_last_branch_msr(msr) )
- wrmsr_hypervisor_regs(msr, msr_content);
+ switch ( wrmsr_hypervisor_regs(msr, msr_content) )
+ {
+ case -EAGAIN:
+ return X86EMUL_RETRY;
+ case 0:
+ case 1:
+ break;
+ default:
+ goto gp_fault;
+ }
break;
case HNDL_exception_raised:
return X86EMUL_EXCEPTION;
diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -634,25 +634,33 @@ int wrmsr_hypervisor_regs(uint32_t idx,
unsigned long gmfn = val >> 12;
unsigned int idx = val & 0xfff;
struct page_info *page;
+ p2m_type_t t;
if ( idx > 0 )
{
gdprintk(XENLOG_WARNING,
"Out of range index %u to MSR %08x\n",
idx, 0x40000000);
- return 0;
+ return -EINVAL;
}
- page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+ page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
if ( !page || !get_page_type(page, PGT_writable_page) )
{
if ( page )
put_page(page);
+
+ if ( p2m_is_paging(t) )
+ {
+ p2m_mem_paging_populate(d, gmfn);
+ return -EAGAIN;
+ }
+
gdprintk(XENLOG_WARNING,
"Bad GMFN %lx (MFN %lx) to MSR %08x\n",
- gmfn, page_to_mfn(page), base + idx);
- return 0;
+ gmfn, page ? page_to_mfn(page) : -1UL, base);
+ return -EINVAL;
}
hypercall_page = __map_domain_page(page);
@@ -2490,7 +2498,7 @@ static int emulate_privileged_op(struct
goto fail;
break;
default:
- if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) )
+ if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) == 1 )
break;
rc = vmce_wrmsr(regs->ecx, msr_content);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 14:31 ` Jan Beulich
@ 2013-05-03 15:19 ` Olaf Hering
2013-05-03 15:55 ` Keir Fraser
0 siblings, 1 reply; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 15:19 UTC (permalink / raw)
To: Jan Beulich; +Cc: Keir Fraser, xen-devel
On Fri, May 03, Jan Beulich wrote:
> >>> On 03.05.13 at 16:26, Keir Fraser <keir.xen@gmail.com> wrote:
> > I suggest the following return codes:
> > 0: not handled
> > 1: handled
> > -EINVAL: error during handling
> > -EAGAIN: retry
> >
> > The HVM callers should then handle as follows:
> > -EAGAIN: rc = X86EMUL_RETRY
> > -EINVAL: goto gp_fault
> > 0: try other msr handlers (if any)
> > 1: we're done, return X86EMUL_OKAY
> >
> > Does that make sense?
>
> Sure - you may have seen my later reply where I also notide
> this mistake in my first response to Olaf.
I sent v3 of the patch. I had to guess which 'return 0' is supposed to
be an error.
Olaf
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 15:17 ` [PATCH v3] " Olaf Hering
@ 2013-05-03 15:30 ` Jan Beulich
2013-05-03 15:48 ` Olaf Hering
2013-05-03 15:58 ` Keir Fraser
2013-05-03 15:55 ` Keir Fraser
1 sibling, 2 replies; 27+ messages in thread
From: Jan Beulich @ 2013-05-03 15:30 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
>>> On 03.05.13 at 17:17, Olaf Hering <olaf@aepfle.de> wrote:
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1569,7 +1569,7 @@ static int svm_msr_read_intercept(unsign
>
> static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
> {
> - int ret;
> + int ret, result = X86EMUL_OKAY;
> struct vcpu *v = current;
> struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> int sync = 0;
> @@ -1682,14 +1682,24 @@ static int svm_msr_write_intercept(unsig
> if ( wrmsr_viridian_regs(msr, msr_content) )
> break;
>
> - wrmsr_hypervisor_regs(msr, msr_content);
> + switch ( wrmsr_hypervisor_regs(msr, msr_content) )
> + {
> + case -EAGAIN:
> + result = X86EMUL_RETRY;
> + break;
> + case 0:
> + case 1:
> + break;
> + default:
> + goto gpf;
> + }
> break;
> }
>
> if ( sync )
> svm_vmload(vmcb);
>
> - return X86EMUL_OKAY;
> + return result;
>
> gpf:
> hvm_inject_hw_exception(TRAP_gp_fault, 0);
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2088,7 +2088,16 @@ static int vmx_msr_write_intercept(unsig
> case HNDL_unhandled:
> if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
> !is_last_branch_msr(msr) )
> - wrmsr_hypervisor_regs(msr, msr_content);
> + switch ( wrmsr_hypervisor_regs(msr, msr_content) )
> + {
> + case -EAGAIN:
> + return X86EMUL_RETRY;
> + case 0:
> + case 1:
> + break;
> + default:
> + goto gp_fault;
> + }
> break;
> case HNDL_exception_raised:
> return X86EMUL_EXCEPTION;
Apart from formatting things look okay up to here.
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -634,25 +634,33 @@ int wrmsr_hypervisor_regs(uint32_t idx,
> unsigned long gmfn = val >> 12;
> unsigned int idx = val & 0xfff;
> struct page_info *page;
> + p2m_type_t t;
>
> if ( idx > 0 )
> {
> gdprintk(XENLOG_WARNING,
> "Out of range index %u to MSR %08x\n",
> idx, 0x40000000);
> - return 0;
> + return -EINVAL;
But I'd stay away from converting to actual errors both here ...
> }
>
> - page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> + page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
>
> if ( !page || !get_page_type(page, PGT_writable_page) )
> {
> if ( page )
> put_page(page);
> +
> + if ( p2m_is_paging(t) )
> + {
> + p2m_mem_paging_populate(d, gmfn);
> + return -EAGAIN;
> + }
> +
> gdprintk(XENLOG_WARNING,
> "Bad GMFN %lx (MFN %lx) to MSR %08x\n",
> - gmfn, page_to_mfn(page), base + idx);
> - return 0;
> + gmfn, page ? page_to_mfn(page) : -1UL, base);
> + return -EINVAL;
... and here. If at all these ought to go into a separate patch
(which we'd likely postpone until after 4.3).
Jan
> }
>
> hypercall_page = __map_domain_page(page);
> @@ -2490,7 +2498,7 @@ static int emulate_privileged_op(struct
> goto fail;
> break;
> default:
> - if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) )
> + if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) == 1 )
> break;
>
> rc = vmce_wrmsr(regs->ecx, msr_content);
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v4] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-02 16:24 [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs Olaf Hering
` (2 preceding siblings ...)
2013-05-03 15:17 ` [PATCH v3] " Olaf Hering
@ 2013-05-03 15:43 ` Olaf Hering
2013-05-03 16:29 ` [PATCH v5] " Olaf Hering
4 siblings, 0 replies; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 15:43 UTC (permalink / raw)
To: xen-devel
# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1367595698 -7200
# Node ID 4b7f3f754c015e0c2012d4b8ef4faf89ba18a3bf
# Parent 9df019eef776d129c2abb130d1458914fe1ecac4
xen: handle paged gfn in wrmsr_hypervisor_regs
If xenpaging is started very early for a guest the gfn for the hypercall
page may be paged-out already. This leads to a guest crash:
...
(XEN) HVM10: Allocated Xen hypercall page at 169ff000
(XEN) traps.c:654:d10 Bad GMFN 169ff (MFN 3e900000000) to MSR 40000000
(XEN) HVM10: Detected Xen v4.3
(XEN) io.c:201:d10 MMIO emulation failed @ 0008:c2c2c2c2: 18 7c 55 6d 03 83 ff ff 10 7c
(XEN) hvm.c:1253:d10 Triple fault on VCPU0 - invoking HVM shutdown action 1.
(XEN) HVM11: HVM Loader
...
Update return codes of wrmsr_hypervisor_regs, update callers to deal
with the new return codes:
0: not handled
1: handled
-EINVAL: error during handling
-EAGAIN: retry
Also update the gdprintk to handle a page value of NULL to avoid
printing a bogus MFN value. Update also computing of MSR value in
gdprintk, the idx was always zero.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
diff -r 9df019eef776 -r 4b7f3f754c01 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1569,7 +1569,7 @@ static int svm_msr_read_intercept(unsign
static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
- int ret;
+ int ret, result = X86EMUL_OKAY;
struct vcpu *v = current;
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
int sync = 0;
@@ -1682,14 +1682,24 @@ static int svm_msr_write_intercept(unsig
if ( wrmsr_viridian_regs(msr, msr_content) )
break;
- wrmsr_hypervisor_regs(msr, msr_content);
+ switch ( wrmsr_hypervisor_regs(msr, msr_content) )
+ {
+ case -EAGAIN:
+ result = X86EMUL_RETRY;
+ break;
+ case 0:
+ case 1:
+ break;
+ default:
+ goto gpf;
+ }
break;
}
if ( sync )
svm_vmload(vmcb);
- return X86EMUL_OKAY;
+ return result;
gpf:
hvm_inject_hw_exception(TRAP_gp_fault, 0);
diff -r 9df019eef776 -r 4b7f3f754c01 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2088,7 +2088,16 @@ static int vmx_msr_write_intercept(unsig
case HNDL_unhandled:
if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
!is_last_branch_msr(msr) )
- wrmsr_hypervisor_regs(msr, msr_content);
+ switch ( wrmsr_hypervisor_regs(msr, msr_content) )
+ {
+ case -EAGAIN:
+ return X86EMUL_RETRY;
+ case 0:
+ case 1:
+ break;
+ default:
+ goto gp_fault;
+ }
break;
case HNDL_exception_raised:
return X86EMUL_EXCEPTION;
diff -r 9df019eef776 -r 4b7f3f754c01 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -634,6 +634,7 @@ int wrmsr_hypervisor_regs(uint32_t idx,
unsigned long gmfn = val >> 12;
unsigned int idx = val & 0xfff;
struct page_info *page;
+ p2m_type_t t;
if ( idx > 0 )
{
@@ -643,15 +644,22 @@ int wrmsr_hypervisor_regs(uint32_t idx,
return 0;
}
- page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+ page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
if ( !page || !get_page_type(page, PGT_writable_page) )
{
if ( page )
put_page(page);
+
+ if ( p2m_is_paging(t) )
+ {
+ p2m_mem_paging_populate(d, gmfn);
+ return -EAGAIN;
+ }
+
gdprintk(XENLOG_WARNING,
"Bad GMFN %lx (MFN %lx) to MSR %08x\n",
- gmfn, page_to_mfn(page), base + idx);
+ gmfn, page ? page_to_mfn(page) : -1UL, base);
return 0;
}
@@ -2490,7 +2498,7 @@ static int emulate_privileged_op(struct
goto fail;
break;
default:
- if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) )
+ if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) == 1 )
break;
rc = vmce_wrmsr(regs->ecx, msr_content);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 15:30 ` Jan Beulich
@ 2013-05-03 15:48 ` Olaf Hering
2013-05-03 15:53 ` Jan Beulich
2013-05-03 15:58 ` Keir Fraser
1 sibling, 1 reply; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 15:48 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Fri, May 03, Jan Beulich wrote:
> Apart from formatting things look okay up to here.
Did you mean just the single tab in svm_msr_write_intercept?
> > --- a/xen/arch/x86/traps.c
> > +++ b/xen/arch/x86/traps.c
> > @@ -634,25 +634,33 @@ int wrmsr_hypervisor_regs(uint32_t idx,
> > unsigned long gmfn = val >> 12;
> > unsigned int idx = val & 0xfff;
> > struct page_info *page;
> > + p2m_type_t t;
> >
> > if ( idx > 0 )
> > {
> > gdprintk(XENLOG_WARNING,
> > "Out of range index %u to MSR %08x\n",
> > idx, 0x40000000);
> > - return 0;
> > + return -EINVAL;
>
> But I'd stay away from converting to actual errors both here ...
I sent v4 with the -EINVAL removed.
Olaf
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 15:48 ` Olaf Hering
@ 2013-05-03 15:53 ` Jan Beulich
0 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2013-05-03 15:53 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
>>> On 03.05.13 at 17:48, Olaf Hering <olaf@aepfle.de> wrote:
> On Fri, May 03, Jan Beulich wrote:
>> Apart from formatting things look okay up to here.
>
> Did you mean just the single tab in svm_msr_write_intercept?
That and the too deep indentation of the case labels within the
new switch blocks.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v2] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 15:19 ` Olaf Hering
@ 2013-05-03 15:55 ` Keir Fraser
0 siblings, 0 replies; 27+ messages in thread
From: Keir Fraser @ 2013-05-03 15:55 UTC (permalink / raw)
To: Olaf Hering, Jan Beulich; +Cc: xen-devel
On 03/05/2013 16:19, "Olaf Hering" <olaf@aepfle.de> wrote:
> On Fri, May 03, Jan Beulich wrote:
>
>>>>> On 03.05.13 at 16:26, Keir Fraser <keir.xen@gmail.com> wrote:
>>> I suggest the following return codes:
>>> 0: not handled
>>> 1: handled
>>> -EINVAL: error during handling
>>> -EAGAIN: retry
>>>
>>> The HVM callers should then handle as follows:
>>> -EAGAIN: rc = X86EMUL_RETRY
>>> -EINVAL: goto gp_fault
>>> 0: try other msr handlers (if any)
>>> 1: we're done, return X86EMUL_OKAY
>>>
>>> Does that make sense?
>>
>> Sure - you may have seen my later reply where I also notide
>> this mistake in my first response to Olaf.
>
> I sent v3 of the patch. I had to guess which 'return 0' is supposed to
> be an error.
You guessed correctly ;)
> Olaf
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 15:17 ` [PATCH v3] " Olaf Hering
2013-05-03 15:30 ` Jan Beulich
@ 2013-05-03 15:55 ` Keir Fraser
1 sibling, 0 replies; 27+ messages in thread
From: Keir Fraser @ 2013-05-03 15:55 UTC (permalink / raw)
To: Olaf Hering, xen-devel
On 03/05/2013 16:17, "Olaf Hering" <olaf@aepfle.de> wrote:
> # HG changeset patch
> # User Olaf Hering <olaf@aepfle.de>
> # Date 1367593457 -7200
> # Node ID b8af60cf8282bfddb13cc10e4ffaf0c396a15104
> # Parent 9df019eef776d129c2abb130d1458914fe1ecac4
> xen: handle paged gfn in wrmsr_hypervisor_regs
Acked-by: Keir Fraser <keir@xen.org>
> If xenpaging is started very early for a guest the gfn for the hypercall
> page may be paged-out already. This leads to a guest crash:
>
> ...
> (XEN) HVM10: Allocated Xen hypercall page at 169ff000
> (XEN) traps.c:654:d10 Bad GMFN 169ff (MFN 3e900000000) to MSR 40000000
> (XEN) HVM10: Detected Xen v4.3
> (XEN) io.c:201:d10 MMIO emulation failed @ 0008:c2c2c2c2: 18 7c 55 6d 03 83 ff
> ff 10 7c
> (XEN) hvm.c:1253:d10 Triple fault on VCPU0 - invoking HVM shutdown action 1.
> (XEN) HVM11: HVM Loader
> ...
>
> Update return codes of wrmsr_hypervisor_regs, update callers to deal
> with the new return codes:
> 0: not handled
> 1: handled
> -EINVAL: error during handling
> -EAGAIN: retry
>
>
> Also update the gdprintk to handle a page value of NULL to avoid
> printing a bogus MFN value. Update also computing of MSR value in
> gdprintk, the idx was always zero.
>
> Signed-off-by: Olaf Hering <olaf@aepfle.de>
>
> diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/hvm/svm/svm.c
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -1569,7 +1569,7 @@ static int svm_msr_read_intercept(unsign
>
> static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
> {
> - int ret;
> + int ret, result = X86EMUL_OKAY;
> struct vcpu *v = current;
> struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> int sync = 0;
> @@ -1682,14 +1682,24 @@ static int svm_msr_write_intercept(unsig
> if ( wrmsr_viridian_regs(msr, msr_content) )
> break;
>
> - wrmsr_hypervisor_regs(msr, msr_content);
> + switch ( wrmsr_hypervisor_regs(msr, msr_content) )
> + {
> + case -EAGAIN:
> + result = X86EMUL_RETRY;
> + break;
> + case 0:
> + case 1:
> + break;
> + default:
> + goto gpf;
> + }
> break;
> }
>
> if ( sync )
> svm_vmload(vmcb);
>
> - return X86EMUL_OKAY;
> + return result;
>
> gpf:
> hvm_inject_hw_exception(TRAP_gp_fault, 0);
> diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/hvm/vmx/vmx.c
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -2088,7 +2088,16 @@ static int vmx_msr_write_intercept(unsig
> case HNDL_unhandled:
> if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
> !is_last_branch_msr(msr) )
> - wrmsr_hypervisor_regs(msr, msr_content);
> + switch ( wrmsr_hypervisor_regs(msr, msr_content) )
> + {
> + case -EAGAIN:
> + return X86EMUL_RETRY;
> + case 0:
> + case 1:
> + break;
> + default:
> + goto gp_fault;
> + }
> break;
> case HNDL_exception_raised:
> return X86EMUL_EXCEPTION;
> diff -r 9df019eef776 -r b8af60cf8282 xen/arch/x86/traps.c
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -634,25 +634,33 @@ int wrmsr_hypervisor_regs(uint32_t idx,
> unsigned long gmfn = val >> 12;
> unsigned int idx = val & 0xfff;
> struct page_info *page;
> + p2m_type_t t;
>
> if ( idx > 0 )
> {
> gdprintk(XENLOG_WARNING,
> "Out of range index %u to MSR %08x\n",
> idx, 0x40000000);
> - return 0;
> + return -EINVAL;
> }
>
> - page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
> + page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
>
> if ( !page || !get_page_type(page, PGT_writable_page) )
> {
> if ( page )
> put_page(page);
> +
> + if ( p2m_is_paging(t) )
> + {
> + p2m_mem_paging_populate(d, gmfn);
> + return -EAGAIN;
> + }
> +
> gdprintk(XENLOG_WARNING,
> "Bad GMFN %lx (MFN %lx) to MSR %08x\n",
> - gmfn, page_to_mfn(page), base + idx);
> - return 0;
> + gmfn, page ? page_to_mfn(page) : -1UL, base);
> + return -EINVAL;
> }
>
> hypercall_page = __map_domain_page(page);
> @@ -2490,7 +2498,7 @@ static int emulate_privileged_op(struct
> goto fail;
> break;
> default:
> - if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) )
> + if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) == 1 )
> break;
>
> rc = vmce_wrmsr(regs->ecx, msr_content);
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 15:30 ` Jan Beulich
2013-05-03 15:48 ` Olaf Hering
@ 2013-05-03 15:58 ` Keir Fraser
2013-05-03 16:03 ` Jan Beulich
1 sibling, 1 reply; 27+ messages in thread
From: Keir Fraser @ 2013-05-03 15:58 UTC (permalink / raw)
To: Jan Beulich, Olaf Hering; +Cc: xen-devel
On 03/05/2013 16:30, "Jan Beulich" <JBeulich@suse.com> wrote:
>> if ( idx > 0 )
>> {
>> gdprintk(XENLOG_WARNING,
>> "Out of range index %u to MSR %08x\n",
>> idx, 0x40000000);
>> - return 0;
>> + return -EINVAL;
>
> But I'd stay away from converting to actual errors both here ...
>
>> }
>>
>> - page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
>> + page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
>>
>> if ( !page || !get_page_type(page, PGT_writable_page) )
>> {
>> if ( page )
>> put_page(page);
>> +
>> + if ( p2m_is_paging(t) )
>> + {
>> + p2m_mem_paging_populate(d, gmfn);
>> + return -EAGAIN;
>> + }
>> +
>> gdprintk(XENLOG_WARNING,
>> "Bad GMFN %lx (MFN %lx) to MSR %08x\n",
>> - gmfn, page_to_mfn(page), base + idx);
>> - return 0;
>> + gmfn, page ? page_to_mfn(page) : -1UL, base);
>> + return -EINVAL;
>
> ... and here. If at all these ought to go into a separate patch
> (which we'd likely postpone until after 4.3).
But they are errors?
Could agree with postponing to post-4.3 though.
-- Keir
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 15:58 ` Keir Fraser
@ 2013-05-03 16:03 ` Jan Beulich
2013-05-03 16:06 ` Keir Fraser
0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2013-05-03 16:03 UTC (permalink / raw)
To: Olaf Hering, Keir Fraser; +Cc: xen-devel
>>> On 03.05.13 at 17:58, Keir Fraser <keir.xen@gmail.com> wrote:
> On 03/05/2013 16:30, "Jan Beulich" <JBeulich@suse.com> wrote:
>
>>> if ( idx > 0 )
>>> {
>>> gdprintk(XENLOG_WARNING,
>>> "Out of range index %u to MSR %08x\n",
>>> idx, 0x40000000);
>>> - return 0;
>>> + return -EINVAL;
>>
>> But I'd stay away from converting to actual errors both here ...
>>
>>> }
>>>
>>> - page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
>>> + page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
>>>
>>> if ( !page || !get_page_type(page, PGT_writable_page) )
>>> {
>>> if ( page )
>>> put_page(page);
>>> +
>>> + if ( p2m_is_paging(t) )
>>> + {
>>> + p2m_mem_paging_populate(d, gmfn);
>>> + return -EAGAIN;
>>> + }
>>> +
>>> gdprintk(XENLOG_WARNING,
>>> "Bad GMFN %lx (MFN %lx) to MSR %08x\n",
>>> - gmfn, page_to_mfn(page), base + idx);
>>> - return 0;
>>> + gmfn, page ? page_to_mfn(page) : -1UL, base);
>>> + return -EINVAL;
>>
>> ... and here. If at all these ought to go into a separate patch
>> (which we'd likely postpone until after 4.3).
>
> But they are errors?
Right. But they weren't handled as such so far, and doing the
conversion isn't the purpose of the patch.
> Could agree with postponing to post-4.3 though.
Hence the request to split off that part of the patch. I'd be happy
to apply that after the trees got branched.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v3] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 16:03 ` Jan Beulich
@ 2013-05-03 16:06 ` Keir Fraser
0 siblings, 0 replies; 27+ messages in thread
From: Keir Fraser @ 2013-05-03 16:06 UTC (permalink / raw)
To: Jan Beulich, Olaf Hering; +Cc: xen-devel
On 03/05/2013 17:03, "Jan Beulich" <JBeulich@suse.com> wrote:
>>> ... and here. If at all these ought to go into a separate patch
>>> (which we'd likely postpone until after 4.3).
>>
>> But they are errors?
>
> Right. But they weren't handled as such so far, and doing the
> conversion isn't the purpose of the patch.
Okay, I wasn't sure how dismissive your "if at all" was. :)
>> Could agree with postponing to post-4.3 though.
>
> Hence the request to split off that part of the patch. I'd be happy
> to apply that after the trees got branched.
Fine.
-- Keir
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH v5] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-02 16:24 [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs Olaf Hering
` (3 preceding siblings ...)
2013-05-03 15:43 ` [PATCH v4] " Olaf Hering
@ 2013-05-03 16:29 ` Olaf Hering
2013-05-08 9:27 ` Jan Beulich
4 siblings, 1 reply; 27+ messages in thread
From: Olaf Hering @ 2013-05-03 16:29 UTC (permalink / raw)
To: xen-devel
# HG changeset patch
# User Olaf Hering <olaf@aepfle.de>
# Date 1367598534 -7200
# Node ID e04344e05ce4ab608df4570d7c081770cc40f3cf
# Parent 9df019eef776d129c2abb130d1458914fe1ecac4
xen: handle paged gfn in wrmsr_hypervisor_regs
If xenpaging is started very early for a guest the gfn for the hypercall
page may be paged-out already. This leads to a guest crash:
...
(XEN) HVM10: Allocated Xen hypercall page at 169ff000
(XEN) traps.c:654:d10 Bad GMFN 169ff (MFN 3e900000000) to MSR 40000000
(XEN) HVM10: Detected Xen v4.3
(XEN) io.c:201:d10 MMIO emulation failed @ 0008:c2c2c2c2: 18 7c 55 6d 03 83 ff ff 10 7c
(XEN) hvm.c:1253:d10 Triple fault on VCPU0 - invoking HVM shutdown action 1.
(XEN) HVM11: HVM Loader
...
Update return codes of wrmsr_hypervisor_regs, update callers to deal
with the new return codes:
0: not handled
1: handled
-EAGAIN: retry
Currently wrmsr_hypervisor_regs will not return the following error, it
will be added in a separate patch:
-EINVAL: error during handling
Also update the gdprintk to handle a page value of NULL to avoid
printing a bogus MFN value. Update also computing of MSR value in
gdprintk, the idx was always zero.
Signed-off-by: Olaf Hering <olaf@aepfle.de>
Acked-by: Keir Fraser <keir@xen.org>
diff -r 9df019eef776 -r e04344e05ce4 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1569,7 +1569,7 @@ static int svm_msr_read_intercept(unsign
static int svm_msr_write_intercept(unsigned int msr, uint64_t msr_content)
{
- int ret;
+ int ret, result = X86EMUL_OKAY;
struct vcpu *v = current;
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
int sync = 0;
@@ -1682,14 +1682,24 @@ static int svm_msr_write_intercept(unsig
if ( wrmsr_viridian_regs(msr, msr_content) )
break;
- wrmsr_hypervisor_regs(msr, msr_content);
+ switch ( wrmsr_hypervisor_regs(msr, msr_content) )
+ {
+ case -EAGAIN:
+ result = X86EMUL_RETRY;
+ break;
+ case 0:
+ case 1:
+ break;
+ default:
+ goto gpf;
+ }
break;
}
if ( sync )
svm_vmload(vmcb);
- return X86EMUL_OKAY;
+ return result;
gpf:
hvm_inject_hw_exception(TRAP_gp_fault, 0);
diff -r 9df019eef776 -r e04344e05ce4 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -2088,7 +2088,16 @@ static int vmx_msr_write_intercept(unsig
case HNDL_unhandled:
if ( (vmx_write_guest_msr(msr, msr_content) != 0) &&
!is_last_branch_msr(msr) )
- wrmsr_hypervisor_regs(msr, msr_content);
+ switch ( wrmsr_hypervisor_regs(msr, msr_content) )
+ {
+ case -EAGAIN:
+ return X86EMUL_RETRY;
+ case 0:
+ case 1:
+ break;
+ default:
+ goto gp_fault;
+ }
break;
case HNDL_exception_raised:
return X86EMUL_EXCEPTION;
diff -r 9df019eef776 -r e04344e05ce4 xen/arch/x86/traps.c
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -634,6 +634,7 @@ int wrmsr_hypervisor_regs(uint32_t idx,
unsigned long gmfn = val >> 12;
unsigned int idx = val & 0xfff;
struct page_info *page;
+ p2m_type_t t;
if ( idx > 0 )
{
@@ -643,15 +644,22 @@ int wrmsr_hypervisor_regs(uint32_t idx,
return 0;
}
- page = get_page_from_gfn(d, gmfn, NULL, P2M_ALLOC);
+ page = get_page_from_gfn(d, gmfn, &t, P2M_ALLOC);
if ( !page || !get_page_type(page, PGT_writable_page) )
{
if ( page )
put_page(page);
+
+ if ( p2m_is_paging(t) )
+ {
+ p2m_mem_paging_populate(d, gmfn);
+ return -EAGAIN;
+ }
+
gdprintk(XENLOG_WARNING,
"Bad GMFN %lx (MFN %lx) to MSR %08x\n",
- gmfn, page_to_mfn(page), base + idx);
+ gmfn, page ? page_to_mfn(page) : -1UL, base);
return 0;
}
@@ -2490,7 +2498,7 @@ static int emulate_privileged_op(struct
goto fail;
break;
default:
- if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) )
+ if ( wrmsr_hypervisor_regs(regs->ecx, msr_content) == 1 )
break;
rc = vmce_wrmsr(regs->ecx, msr_content);
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-03 16:29 ` [PATCH v5] " Olaf Hering
@ 2013-05-08 9:27 ` Jan Beulich
2013-05-08 10:51 ` Olaf Hering
0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2013-05-08 9:27 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
>>> On 03.05.13 at 18:29, Olaf Hering <olaf@aepfle.de> wrote:
> xen: handle paged gfn in wrmsr_hypervisor_regs
Does this need any backporting?
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-08 9:27 ` Jan Beulich
@ 2013-05-08 10:51 ` Olaf Hering
2013-05-08 11:41 ` Jan Beulich
0 siblings, 1 reply; 27+ messages in thread
From: Olaf Hering @ 2013-05-08 10:51 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Wed, May 08, Jan Beulich wrote:
> >>> On 03.05.13 at 18:29, Olaf Hering <olaf@aepfle.de> wrote:
> > xen: handle paged gfn in wrmsr_hypervisor_regs
>
> Does this need any backporting?
4.2 has the same issue, if I read the code correctly.
Olaf
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-08 10:51 ` Olaf Hering
@ 2013-05-08 11:41 ` Jan Beulich
2013-05-08 11:45 ` Olaf Hering
0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2013-05-08 11:41 UTC (permalink / raw)
To: Olaf Hering; +Cc: xen-devel
>>> On 08.05.13 at 12:51, Olaf Hering <olaf@aepfle.de> wrote:
> On Wed, May 08, Jan Beulich wrote:
>
>> >>> On 03.05.13 at 18:29, Olaf Hering <olaf@aepfle.de> wrote:
>> > xen: handle paged gfn in wrmsr_hypervisor_regs
>>
>> Does this need any backporting?
>
> 4.2 has the same issue, if I read the code correctly.
I understand that - the question is whether paging is usable
enough in 4.2 to warrant this being backported.
Jan
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH v5] xen: handle paged gfn in wrmsr_hypervisor_regs
2013-05-08 11:41 ` Jan Beulich
@ 2013-05-08 11:45 ` Olaf Hering
0 siblings, 0 replies; 27+ messages in thread
From: Olaf Hering @ 2013-05-08 11:45 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel
On Wed, May 08, Jan Beulich wrote:
> >>> On 08.05.13 at 12:51, Olaf Hering <olaf@aepfle.de> wrote:
> > On Wed, May 08, Jan Beulich wrote:
> >
> >> >>> On 03.05.13 at 18:29, Olaf Hering <olaf@aepfle.de> wrote:
> >> > xen: handle paged gfn in wrmsr_hypervisor_regs
> >>
> >> Does this need any backporting?
> >
> > 4.2 has the same issue, if I read the code correctly.
>
> I understand that - the question is whether paging is usable
> enough in 4.2 to warrant this being backported.
It is, but for some reason I did not run into this issue.
Olaf
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2013-05-08 11:45 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-02 16:24 [PATCH] xen: handle paged gfn in wrmsr_hypervisor_regs Olaf Hering
2013-05-03 7:12 ` Jan Beulich
2013-05-03 12:58 ` Olaf Hering
2013-05-03 13:40 ` Olaf Hering
2013-05-03 13:53 ` Jan Beulich
2013-05-03 12:57 ` [PATCH v2] " Olaf Hering
2013-05-03 13:58 ` Jan Beulich
2013-05-03 14:11 ` Olaf Hering
2013-05-03 14:24 ` Jan Beulich
2013-05-03 14:26 ` Keir Fraser
2013-05-03 14:31 ` Jan Beulich
2013-05-03 15:19 ` Olaf Hering
2013-05-03 15:55 ` Keir Fraser
2013-05-03 15:17 ` [PATCH v3] " Olaf Hering
2013-05-03 15:30 ` Jan Beulich
2013-05-03 15:48 ` Olaf Hering
2013-05-03 15:53 ` Jan Beulich
2013-05-03 15:58 ` Keir Fraser
2013-05-03 16:03 ` Jan Beulich
2013-05-03 16:06 ` Keir Fraser
2013-05-03 15:55 ` Keir Fraser
2013-05-03 15:43 ` [PATCH v4] " Olaf Hering
2013-05-03 16:29 ` [PATCH v5] " Olaf Hering
2013-05-08 9:27 ` Jan Beulich
2013-05-08 10:51 ` Olaf Hering
2013-05-08 11:41 ` Jan Beulich
2013-05-08 11:45 ` Olaf Hering
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.