Gleb Natapov <gleb@xxxxxxxxxx> wrote on 14/04/2013 02:16:02 PM: > On Sun, Apr 14, 2013 at 01:49:44PM +0300, Abel Gordon wrote: > > > > > > Gleb Natapov <gleb@xxxxxxxxxx> wrote on 14/04/2013 01:34:52 PM: > > > > > On Sun, Apr 14, 2013 at 12:27:10PM +0200, Jan Kiszka wrote: > > > > On 2013-04-14 12:07, Gleb Natapov wrote: > > > > > On Sun, Apr 14, 2013 at 01:00:10PM +0300, Gleb Natapov wrote: > > > > >> On Sun, Apr 14, 2013 at 12:51:34PM +0300, Abel Gordon wrote: > > > > >>> > > > > >>> > > > > >>> Gleb Natapov <gleb@xxxxxxxxxx> wrote on 12/04/2013 01:48:04 PM: > > > > >>> > > > > >>>> On Fri, Apr 12, 2013 at 01:44:14PM +0300, Abel Gordon wrote: > > > > >>>>> > > > > >>>>> Ok, so then you prefer to add the inline functions to read/ > > > write to the > > > > >>>>> vmcs12 > > > > >>>>> fields, (to set the request bit if shadowed field changed) and > > you are > > > > >>> not > > > > >>>>> concerned > > > > >>>>> about any merge/rebase mess. I will work on this direction. > > > > >>>>> I'll first send an independent patch to introduce the accessors. > > Once > > > > >>> you > > > > >>>>> apply this patch, I'll continue and send you v2 patches for > > shadow > > > > >>> vmcs. > > > > >>>>> > > > > >>>>> Do you agree ? > > > > >>>> Yes. > > > > >>> > > > > >>> Looking again at the code it seems like we could avoid adding the > > > > >>> accessors. > > > > >>> We could just set a flag in nested_vmx_vmexit and > > > > >>> nested_vmx_entry_failure. Then, in vmx_vcpu_run we check/reset > > > the flag and > > > > >>> call copy_vmcs12_to_shadow (if required). > > > > >>> > > > > >>> What do you think ? > > > > >> Good idea! With accessors we can do further optimization by copying > > only > > > > >> things that changed, but it will be premature optimization at this > > > > >> point. > > > > >> > > > > > Actually this is good idea only if we know for sure that VMX > > emulation > > > > > changes vmcs12 only during guest entry/exit. Is this the case? I > > think > > > > > so. > > > > > > > > Some vmcs12 fields that are exposed to L1 are changed outside L2<-> L1 > > > > transitions. What comes to my mind: L0 emulates some change that L1 > > does > > > > not trap, e.g. CRx accesses. Or what do you mean? > > > > > > > If vmcs12 is changed by L0 while L2 is running this is OK. If L0 changes > > > shadowed vmcs12 field while L1 is running this is not OK. So for > > > instance if field XXX is R/W but we allow only read to be shadowed then > > > write emulation in L0 has to sync new value back to shadow before going > > > back to L1. > > > > Exactly. > > > > While L1 runs (L1 root mode), L0 does NOT change VMCS12 (unless L1 > > executes vmwrite). > > VMCS12 fields are changed once L1 launches/resumes L2 and there is a > > L2 exit. > > > > L0 can change VMCS12 while it handles a L2 exit directly which is not > > forwarded to L1. But that's OK because L1 will eventually see the change > > once we switch to L1 due to other exit that L0 let L1 handle. > > > > L0 should NOT change VMCS12 fields if L1 is running and L1 didn't > > execute any vmlaunch, vmresume or vmwrite instruction. > > > The question is: is there a case like I described above when we shadow > only reads from R/W field and handle vmwrites in L0? No, there is no such thing with the patches I submitted. The shadow VMCS patches assumes that only R/O fields can be shadowed for read and not for write (the shadow_read_only_fields array). R/O fields can't be written by L1 thus L0 will trap vmwrite to any r/o field and emulate a fail (VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT). Writable fields are shadowed for both read and write (the shadow_read_write_fields array). There are no writable fields shadowed for write and not for read. There are no writable fields shadowed for read and not for write -- 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
