Avi Kivity <avi@xxxxxxxxxx> wrote on 20/10/2009 06:44:41:
> From:
>
> Avi Kivity <avi@xxxxxxxxxx>
>
> To:
>
> Orit Wasserman/Haifa/IBM@IBMIL
>
> Cc:
>
> kvm@xxxxxxxxxxxxxxx, Ben-Ami Yassour1/Haifa/IBM@IBMIL, Abel Gordon/
> Haifa/IBM@IBMIL, Muli Ben-Yehuda/Haifa/IBM@IBMIL,
> aliguori@xxxxxxxxxx, mdday@xxxxxxxxxx
>
> Date:
>
> 20/10/2009 06:44
>
> Subject:
>
> Re: [PATCH 4/5] Nested VMX patch 4 implements vmread and vmwrite
>
> On 10/15/2009 11:41 PM, oritw@xxxxxxxxxx wrote:
> >
> > +static int nested_map_shadow_vmcs(struct kvm_vcpu *vcpu)
> > +{
> > + struct vcpu_vmx *vmx = to_vmx(vcpu);
> > + struct page *vmcs_page = nested_get_page(vcpu, vmx->nested.vmptr);
> > +
> > + if (vmcs_page == NULL) {
> > + printk(KERN_INFO "%s: failure in nested_get_page\n",__func__);
> > + return 0;
> > + }
> > +
> > + if (vmx->nested.l2_state->shadow_vmcs) {
> > + printk(KERN_INFO "%s: shadow vmcs already mapped\n",__func__);
> > + return 0;
> > + }
> > +
> >
>
> Consider dropping shadow_vmcs from l2_state and just passing it
> everywhere. Less convenient but safer.
I will think about it, it is called from many places ...
>
> > + vmx->nested.l2_state->shadow_vmcs = kmap_atomic(vmcs_page,
KM_USER0);
> > +
> > + if (!vmx->nested.l2_state->shadow_vmcs) {
> > + printk(KERN_INFO "%s: error in kmap_atomic\n",__func__);
> > + return 0;
> > + }
> >
>
> kmap_atomic() can't fail.
I will remove the check.
> >
> > +static int handle_vmread(struct kvm_vcpu *vcpu)
> > +{
> > +#ifndef CONFIG_X86_64
> > + u64 value;
> > +#endif
> > +
> > + if (!nested_vmx_check_permission(vcpu))
> > + return 1;
> > +
> > + if (!nested_map_shadow_vmcs(vcpu)) {
> > + printk(KERN_INFO "%s invalid shadow vmcs\n", __func__);
> > + set_rflags_to_vmx_fail_invalid(vcpu);
> > + return 1;
> > + }
> >
>
> return an error.
OK.
>
> > +
> > + switch (vmcs_field_length(vcpu->arch.regs[VCPU_REGS_RDX])) {
> > + case VMCS_FIELD_TYPE_U16:
> > + vcpu->arch.regs[VCPU_REGS_RAX] =
> > + nested_vmcs_read16(vcpu,
> > + vcpu->arch.regs[VCPU_REGS_RDX]);
> > + break;
> >
>
> Use the emulator to decode operands.
OK.
>
>
> --
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.
>
--
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
