On Sun, Mar 06, 2011 at 02:55:17PM +0200, Avi Kivity wrote:
> On 03/06/2011 02:42 PM, Gleb Natapov wrote:
> >>
> >IO permission checking for 64-bit guest in KVM is wrong. The patch bellow
> >should fix it.
> >
> >
> >diff --git a/arch/x86/include/asm/kvm_emulate.h b/arch/x86/include/asm/kvm_emulate.h
> >index 50ebc32..7ef5b86 100644
> >--- a/arch/x86/include/asm/kvm_emulate.h
> >+++ b/arch/x86/include/asm/kvm_emulate.h
> >@@ -142,10 +142,8 @@ struct x86_emulate_ops {
> > int (*pio_out_emulated)(int size, unsigned short port, const void *val,
> > unsigned int count, struct kvm_vcpu *vcpu);
> >
> >- bool (*get_cached_descriptor)(struct desc_struct *desc,
> >- int seg, struct kvm_vcpu *vcpu);
> >- void (*set_cached_descriptor)(struct desc_struct *desc,
> >- int seg, struct kvm_vcpu *vcpu);
> >+ bool (*get_cached_descriptor)(void *p, int seg, struct kvm_vcpu *vcpu);
> >+ void (*set_cached_descriptor)(void *p, int seg, struct kvm_vcpu *vcpu);
>
> Ugh, void *.
>
> Add a u64 *highword parameter, or something.
>
I modeled this after set_tssldt_descriptor() which takes void and cast
it to different structures depending on 64bit/32bit, but u32 *highword
will work too.
> >@@ -1764,25 +1764,35 @@ static bool emulator_io_port_access_allowed(struct x86_emulate_ctxt *ctxt,
> > struct x86_emulate_ops *ops,
> > u16 port, u16 len)
> > {
> >- struct desc_struct tr_seg;
> >+ union {
> >+ struct desc_struct tss32;
> >+#ifdef CONFIG_X86_64
> >+ struct ldttss_desc64 tss64;
> >+#endif
> >+ } tr_seg;
> > int r;
> > u16 io_bitmap_ptr;
> > u8 perm, bit_idx = port& 0x7;
> > unsigned mask = (1<< len) - 1;
> >+ unsigned long base;
> >
> > ops->get_cached_descriptor(&tr_seg, VCPU_SREG_TR, ctxt->vcpu);
> >- if (!tr_seg.p)
> >+ if (!tr_seg.tss32.p)
> > return false;
> >- if (desc_limit_scaled(&tr_seg)< 103)
> >+ if (desc_limit_scaled(&tr_seg.tss32)< 103)
> > return false;
> >- r = ops->read_std(get_desc_base(&tr_seg) + 102,&io_bitmap_ptr, 2,
> >- ctxt->vcpu, NULL);
> >+ base = get_desc_base(&tr_seg.tss32);
> >+#ifdef CONFIG_X86_64
> >+ if (ctxt->mode == X86EMUL_MODE_PROT64)
> >+ base |= ((u64)tr_seg.tss64.base3)<< 32;
> >+#endif
> >+ r = ops->read_std(base + 102,&io_bitmap_ptr, 2, ctxt->vcpu, NULL);
> > if (r != X86EMUL_CONTINUE)
> > return false;
>
> Note: if we get a fault here, we ought to propagate it. Only
> happens if there's a race, since the cpu checks for these
> exceptions.
>
Do you mean we need to propagate fault that read_std() returns and not
always inject GP like we do now?
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
