|
On 11/11/13 11:07, Jan Beulich wrote:
For one, nestedsvm_vmcb_map() error checking must not consist of using
assertions: Global (permanent) mappings can fail, and hence failure
needs to be dealt with properly. And non-global (transient) mappings
can't fail anyway.
And then the I/O port access bitmap handling was broken: It checked
only to first of the accessed ports rather than each of them.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
v2: Refine the change to nsvm_vmrun_permissionmap() -
hvm_map_guest_frame_*() can return NULL for reasons other than
map_domain_page_global() doing so (pointed out by Andrew Cooper).
That leaves the function broken though (it ought to handle the
shared/paged out cases in an appropriate way).
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -342,7 +342,7 @@ static int nsvm_vmrun_permissionmap(stru
unsigned int i;
enum hvm_copy_result ret;
unsigned long *ns_viomap;
- bool_t ioport_80, ioport_ed;
+ bool_t ioport_80 = 1, ioport_ed = 1;
ns_msrpm_ptr = (unsigned long *)svm->ns_cached_msrpm;
@@ -360,10 +360,12 @@ static int nsvm_vmrun_permissionmap(stru
svm->ns_iomap_pa = ns_vmcb->_iopm_base_pa;
ns_viomap = hvm_map_guest_frame_ro(svm->ns_iomap_pa >> PAGE_SHIFT, 0);
- ASSERT(ns_viomap != NULL);
- ioport_80 = test_bit(0x80, ns_viomap);
- ioport_ed = test_bit(0xed, ns_viomap);
- hvm_unmap_guest_frame(ns_viomap, 0);
+ if ( ns_viomap )
+ {
+ ioport_80 = test_bit(0x80, ns_viomap);
+ ioport_ed = test_bit(0xed, ns_viomap);
+ hvm_unmap_guest_frame(ns_viomap, 0);
+ }
Should we not bail on an error here, similar to the failure of
hvm_copy_from_guest just out of context above?
svm->ns_iomap = nestedhvm_vcpu_iomap_get(ioport_80, ioport_ed);
@@ -866,40 +868,45 @@ nsvm_vmcb_guest_intercepts_msr(unsigned
static int
nsvm_vmcb_guest_intercepts_ioio(paddr_t iopm_pa, uint64_t exitinfo1)
{
- unsigned long iopm_gfn = iopm_pa >> PAGE_SHIFT;
- unsigned long *io_bitmap = NULL;
+ unsigned long gfn = iopm_pa >> PAGE_SHIFT;
+ unsigned long *io_bitmap;
ioio_info_t ioinfo;
uint16_t port;
+ unsigned int size;
bool_t enabled;
- unsigned long gfn = 0; /* gcc ... */
ioinfo.bytes = exitinfo1;
port = ioinfo.fields.port;
+ size = ioinfo.fields.sz32 ? 4 : ioinfo.fields.sz16 ? 2 : 1;
- switch (port) {
- case 0 ... 32767: /* first 4KB page */
- gfn = iopm_gfn;
+ switch ( port )
+ {
+ case 0 ... 8 * PAGE_SIZE - 1: /* first 4KB page */
break;
- case 32768 ... 65535: /* second 4KB page */
- port -= 32768;
- gfn = iopm_gfn + 1;
+ case 8 * PAGE_SIZE ... 2 * 8 * PAGE_SIZE - 1: /* second 4KB page */
+ port -= 8 * PAGE_SIZE;
+ ++gfn;
break;
default:
BUG();
break;
}
- io_bitmap = hvm_map_guest_frame_ro(gfn, 0);
- if (io_bitmap == NULL) {
- gdprintk(XENLOG_ERR,
- "IOIO intercept: mapping of permission map failed\n");
- return NESTEDHVM_VMEXIT_ERROR;
+ for ( io_bitmap = hvm_map_guest_frame_ro(gfn, 0); ; )
This needs further checking for NULL.
~Andrew
+ {
+ enabled = test_bit(port, io_bitmap);
+ if ( !enabled || !--size )
+ break;
+ if ( unlikely(++port == 8 * PAGE_SIZE) )
+ {
+ hvm_unmap_guest_frame(io_bitmap, 0);
+ io_bitmap = hvm_map_guest_frame_ro(++gfn, 0);
+ port -= 8 * PAGE_SIZE;
+ }
}
-
- enabled = test_bit(port, io_bitmap);
hvm_unmap_guest_frame(io_bitmap, 0);
- if (!enabled)
+ if ( !enabled )
return NESTEDHVM_VMEXIT_HOST;
return NESTEDHVM_VMEXIT_INJECT;
@@ -966,8 +973,8 @@ nsvm_vmcb_guest_intercepts_exitcode(stru
switch (exitcode) {
case VMEXIT_MSR:
ASSERT(regs != NULL);
- nestedsvm_vmcb_map(v, nv->nv_vvmcxaddr);
- ASSERT(nv->nv_vvmcx != NULL);
+ if ( !nestedsvm_vmcb_map(v, nv->nv_vvmcxaddr) )
+ break;
ns_vmcb = nv->nv_vvmcx;
vmexits = nsvm_vmcb_guest_intercepts_msr(svm->ns_cached_msrpm,
regs->ecx, ns_vmcb->exitinfo1 != 0);
@@ -975,8 +982,8 @@ nsvm_vmcb_guest_intercepts_exitcode(stru
return 0;
break;
case VMEXIT_IOIO:
- nestedsvm_vmcb_map(v, nv->nv_vvmcxaddr);
- ASSERT(nv->nv_vvmcx != NULL);
+ if ( !nestedsvm_vmcb_map(v, nv->nv_vvmcxaddr) )
+ break;
ns_vmcb = nv->nv_vvmcx;
vmexits = nsvm_vmcb_guest_intercepts_ioio(ns_vmcb->_iopm_base_pa,
ns_vmcb->exitinfo1);
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
