Re: [Xen-devel] [PATCH 4/7] x86/hvm: Rename v->arch.hvm_vcpu to v->arch.hvm
> -Original Message- > From: Andrew Cooper [mailto:andrew.coop...@citrix.com] > Sent: 28 August 2018 18:39 > To: Xen-devel > Cc: Andrew Cooper ; Jan Beulich > ; Wei Liu ; Roger Pau Monne > ; Tim (Xen.org) ; George Dunlap > ; Paul Durrant ; > Razvan Cojocaru ; Tamas K Lengyel > ; Stefano Stabellini ; Julien > Grall ; Jun Nakajima ; > Kevin Tian ; Boris Ostrovsky > ; Suravee Suthikulpanit > ; Brian Woods > Subject: [PATCH 4/7] x86/hvm: Rename v->arch.hvm_vcpu to v->arch.hvm > > The trailing _vcpu suffix is redundant, but adds to code volume. Drop it. > > Reflow lines as appropriate, and switch to using the new XFREE/etc wrappers > where applicable. > > No functional change. > > Signed-off-by: Andrew Cooper Acked-by: Paul Durrant ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/7] x86/hvm: Rename v->arch.hvm_vcpu to v->arch.hvm
On 30/08/18 15:52, Jan Beulich wrote: On 28.08.18 at 19:39, wrote: On 28.08.18 at 19:39, wrote: >> The trailing _vcpu suffix is redundant, but adds to code volume. Drop it. >> >> Reflow lines as appropriate, and switch to using the new XFREE/etc wrappers >> where applicable. > I couldn't find any such conversion, so perhaps better to delete that > part of the description. Fixed. > >> No functional change. >> >> Signed-off-by: Andrew Cooper > Reviewed-by: Jan Beulich Thanks. > >> @@ -3888,19 +3886,19 @@ void hvm_vcpu_reset_state(struct vcpu *v, uint16_t >> cs, uint16_t ip) >> v->arch.user_regs.rip = ip; >> memset(>arch.debugreg, 0, sizeof(v->arch.debugreg)); >> >> -v->arch.hvm_vcpu.guest_cr[0] = X86_CR0_ET; >> +v->arch.hvm.guest_cr[0] = X86_CR0_ET; >> hvm_update_guest_cr(v, 0); >> >> -v->arch.hvm_vcpu.guest_cr[2] = 0; >> +v->arch.hvm.guest_cr[2] = 0; >> hvm_update_guest_cr(v, 2); >> >> -v->arch.hvm_vcpu.guest_cr[3] = 0; >> +v->arch.hvm.guest_cr[3] = 0; >> hvm_update_guest_cr(v, 3); >> >> -v->arch.hvm_vcpu.guest_cr[4] = 0; >> +v->arch.hvm.guest_cr[4] = 0; >> hvm_update_guest_cr(v, 4); >> >> -v->arch.hvm_vcpu.guest_efer = 0; >> +v->arch.hvm.guest_efer = 0; >> hvm_update_guest_efer(v); > Noticing this, a thought unrelated to this series: Wouldn't we be better off > setting all the structure fields first, and only then invoke all the > functions? > Just like arch_set_info_hvm_guest() does? Actually, arch_set_info_hvm_guest() is broken in a related way. When nested virt is in the mix, the usual rules for which control bits can be changed are relaxed, and you can get into a number of corner cases where these helpers don't do the correct thing. (e.g. when a 32bit PAE guest tries vmexiting to a PCID hypervisor). Resolving this mess is on the todo list, which includes this function, and others. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/7] x86/hvm: Rename v->arch.hvm_vcpu to v->arch.hvm
>>> On 28.08.18 at 19:39, wrote: >>> On 28.08.18 at 19:39, wrote: > The trailing _vcpu suffix is redundant, but adds to code volume. Drop it. > > Reflow lines as appropriate, and switch to using the new XFREE/etc wrappers > where applicable. I couldn't find any such conversion, so perhaps better to delete that part of the description. > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich > @@ -3888,19 +3886,19 @@ void hvm_vcpu_reset_state(struct vcpu *v, uint16_t > cs, uint16_t ip) > v->arch.user_regs.rip = ip; > memset(>arch.debugreg, 0, sizeof(v->arch.debugreg)); > > -v->arch.hvm_vcpu.guest_cr[0] = X86_CR0_ET; > +v->arch.hvm.guest_cr[0] = X86_CR0_ET; > hvm_update_guest_cr(v, 0); > > -v->arch.hvm_vcpu.guest_cr[2] = 0; > +v->arch.hvm.guest_cr[2] = 0; > hvm_update_guest_cr(v, 2); > > -v->arch.hvm_vcpu.guest_cr[3] = 0; > +v->arch.hvm.guest_cr[3] = 0; > hvm_update_guest_cr(v, 3); > > -v->arch.hvm_vcpu.guest_cr[4] = 0; > +v->arch.hvm.guest_cr[4] = 0; > hvm_update_guest_cr(v, 4); > > -v->arch.hvm_vcpu.guest_efer = 0; > +v->arch.hvm.guest_efer = 0; > hvm_update_guest_efer(v); Noticing this, a thought unrelated to this series: Wouldn't we be better off setting all the structure fields first, and only then invoke all the functions? Just like arch_set_info_hvm_guest() does? Jan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/7] x86/hvm: Rename v->arch.hvm_vcpu to v->arch.hvm
> From: Andrew Cooper [mailto:andrew.coop...@citrix.com] > Sent: Wednesday, August 29, 2018 1:39 AM > > The trailing _vcpu suffix is redundant, but adds to code volume. Drop it. > > Reflow lines as appropriate, and switch to using the new XFREE/etc > wrappers > where applicable. > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Kevin Tian ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/7] x86/hvm: Rename v->arch.hvm_vcpu to v->arch.hvm
On Tue, Aug 28, 2018 at 06:39:03PM +0100, Andrew Cooper wrote: > The trailing _vcpu suffix is redundant, but adds to code volume. Drop it. > > Reflow lines as appropriate, and switch to using the new XFREE/etc wrappers > where applicable. > > No functional change. > > Signed-off-by: Andrew Cooper Reviewed-by: Wei Liu ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 4/7] x86/hvm: Rename v->arch.hvm_vcpu to v->arch.hvm
On 8/28/18 8:39 PM, Andrew Cooper wrote: > The trailing _vcpu suffix is redundant, but adds to code volume. Drop it. > > Reflow lines as appropriate, and switch to using the new XFREE/etc wrappers > where applicable. > > No functional change. > > Signed-off-by: Andrew Cooper Acked-by: Razvan Cojocaru Thanks, Razvan ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
