Re: [Xen-devel] [PATCH] x86/msr: Fix fallout from mostly c/s 832c180

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Wed, Apr 10, 2019 at 10:41:24AM +0100, Andrew Cooper wrote:
> On 10/04/2019 09:23, Wei Liu wrote:
> > On Tue, Apr 09, 2019 at 06:53:47PM +0100, Andrew Cooper wrote:
> >> The series 832c1803^..f61685a6 was committed without adequate review.
> >>
> >>  * Fix the shim build by providing a !CONFIG_HVM declaration for
> >>    hvm_get_guest_bndcfgs()
> >>  * Revert the bogus de-const'ing of the vcpu pointer in
> >>    vmx_get_guest_bndcfgs().  vmx_vmcs_enter() really does mutate the vcpu, 
> >> and
> >>    may cause it to undergo a full de/reschedule, which is in violation of 
> >> the
> >>    ABI described by hvm_get_guest_bndcfgs().  guest_rdmsr() was always 
> >> going
> >>    to need to lose its const parameter, and this was the correct time for 
> >> it
> >>    to happen.
> > Enlighten me -- why does guest_rdmsr need to lose its const parameter?
> >
> > This reads as if the code was buggy from the get-go.
> 
> It was originally written without a const vcpu *, in full knowledge that
> when moving onto some of the less trivial MSRs, it would need a mutable
> parameter.
> 
> Of course, this wasn't considered a good enough argument during review,
> and to avoid an argument, it went in with a const parameter.
> 
> The state of the MSRs may live in various memory structures, or directly
> in hardware (covers the two cases thus far), or in the VMCS/VMCB, (or
> worse, the MSR load/save lists).  This patch adds the first example of
> needing to access data straight from the VMCS, which involves
> negotiating with the scheduler for safe access.

Thanks for the explanation.

Wei.

> 
> ~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel