Re: [PATCH v4 2/3] x86/vmx: Update DR7 type

[dmkhn@xxxxxxxxx]

On Mon, Apr 28, 2025 at 12:48:42PM +0100, Andrew Cooper wrote:
> On 26/04/2025 8:27 am, dmkhn@xxxxxxxxx wrote:
> > From: Denis Mukhin <dmukhin@xxxxxxxx>
> >
> > Convert the DR7 type to `unsigned int` and fix the accesses where necessary.
> >
> > [1] 
> > https://lore.kernel.org/xen-devel/0d01646b-83e3-4a02-b365-d149d2664e73@xxxxxxxxxx/
> >
> > Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
> > ---
> >  xen/arch/x86/hvm/vmx/vmx.c        | 2 +-
> >  xen/arch/x86/include/asm/domain.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> > index 4883bd823d..75c6992172 100644
> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > @@ -926,7 +926,7 @@ static void vmx_save_dr(struct vcpu *v)
> >      v->arch.dr[3] = read_debugreg(3);
> >      v->arch.dr6   = read_debugreg(6);
> >      /* DR7 must be saved as it is used by vmx_restore_dr(). */
> > -    __vmread(GUEST_DR7, &v->arch.dr7);
> > +    v->arch.dr7 = vmread(GUEST_DR7);
> 
> Two minor points.  The = wants lining up for vertical tabulation, and ...
> 
> >  }
> >
> >  static void __restore_debug_registers(struct vcpu *v)
> > diff --git a/xen/arch/x86/include/asm/domain.h 
> > b/xen/arch/x86/include/asm/domain.h
> > index 5fc1d1e5d0..a54ef3a8c1 100644
> > --- a/xen/arch/x86/include/asm/domain.h
> > +++ b/xen/arch/x86/include/asm/domain.h
> > @@ -595,7 +595,7 @@ struct arch_vcpu
> >
> >      /* Debug registers. */
> >      unsigned long dr[4];
> > -    unsigned long dr7; /* Ideally int, but __vmread() needs long. */
> > +    unsigned int dr7;
> >      unsigned int dr6;
> 
> ... these fields want switching back around now that dr7 is unsigned int.
> 
> We always access in numeric order, and they're only out-of-order for
> packing reasons.
> 
> Can fix on commit.

Thanks!

> 
> ~Andrew