In preparation of making the value somewhat dynamic drop the constant.
Replace its use in guest_wrmsr_x2apic() by actually fetching the LVR
value.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/hvm/vlapic.c
+++ b/xen/arch/x86/hvm/vlapic.c
@@ -31,7 +31,6 @@
 #include <public/hvm/ioreq.h>
 #include <public/hvm/params.h>
 
-#define VLAPIC_VERSION                  0x00050014
 #define LVT_BIAS(reg)                   (((reg) - APIC_LVTT) >> 4)
 
 #define LVTS \
@@ -1015,7 +1014,7 @@ int guest_wrmsr_x2apic(struct vcpu *v, u
     case APIC_SPIV:
         if ( val & ~(APIC_VECTOR_MASK | APIC_SPIV_APIC_ENABLED |
                      APIC_SPIV_FOCUS_DISABLED |
-                     (VLAPIC_VERSION & APIC_LVR_DIRECTED_EOI
+                     (vlapic_get_reg(vlapic, APIC_LVR) & APIC_LVR_DIRECTED_EOI
                       ? APIC_SPIV_DIRECTED_EOI : 0)) )
             return X86EMUL_EXCEPTION;
         break;
@@ -1439,7 +1438,7 @@ static void vlapic_do_init(struct vlapic
     if ( !has_vlapic(vlapic_vcpu(vlapic)->domain) )
         return;
 
-    vlapic_set_reg(vlapic, APIC_LVR, VLAPIC_VERSION);
+    vlapic_set_reg(vlapic, APIC_LVR, 0x00050014);
 
     for ( i = 0; i < 8; i++ )
     {

On Wed, Oct 08, 2025 at 02:08:48PM +0200, Jan Beulich wrote:
> In preparation of making the value somewhat dynamic drop the constant.
> Replace its use in guest_wrmsr_x2apic() by actually fetching the LVR
> value.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -31,7 +31,6 @@
>  #include <public/hvm/ioreq.h>
>  #include <public/hvm/params.h>
>  
> -#define VLAPIC_VERSION                  0x00050014
>  #define LVT_BIAS(reg)                   (((reg) - APIC_LVTT) >> 4)
>  
>  #define LVTS \
> @@ -1015,7 +1014,7 @@ int guest_wrmsr_x2apic(struct vcpu *v, u
>      case APIC_SPIV:
>          if ( val & ~(APIC_VECTOR_MASK | APIC_SPIV_APIC_ENABLED |
>                       APIC_SPIV_FOCUS_DISABLED |
> -                     (VLAPIC_VERSION & APIC_LVR_DIRECTED_EOI
> +                     (vlapic_get_reg(vlapic, APIC_LVR) & APIC_LVR_DIRECTED_EOI
>                        ? APIC_SPIV_DIRECTED_EOI : 0)) )
>              return X86EMUL_EXCEPTION;
>          break;
> @@ -1439,7 +1438,7 @@ static void vlapic_do_init(struct vlapic
>      if ( !has_vlapic(vlapic_vcpu(vlapic)->domain) )
>          return;
>  
> -    vlapic_set_reg(vlapic, APIC_LVR, VLAPIC_VERSION);
> +    vlapic_set_reg(vlapic, APIC_LVR, 0x00050014);

(Maybe I'm getting ahead of patch 3, as I haven't looked yet)

Don't we want to use some kind of macros to build this in a more
friendly way?

We could have a pair of SET_APIC_{VERSION,MAXLVT}()?

Thanks, Roger.

On 08.10.2025 18:23, Roger Pau Monné wrote:
> On Wed, Oct 08, 2025 at 02:08:48PM +0200, Jan Beulich wrote:
>> @@ -1439,7 +1438,7 @@ static void vlapic_do_init(struct vlapic
>>      if ( !has_vlapic(vlapic_vcpu(vlapic)->domain) )
>>          return;
>>  
>> -    vlapic_set_reg(vlapic, APIC_LVR, VLAPIC_VERSION);
>> +    vlapic_set_reg(vlapic, APIC_LVR, 0x00050014);
> 
> (Maybe I'm getting ahead of patch 3, as I haven't looked yet)
> 
> Don't we want to use some kind of macros to build this in a more
> friendly way?
> 
> We could have a pair of SET_APIC_{VERSION,MAXLVT}()?

With what patch 3 does to apicdef.h, I was rather wondering whether to simply
use two MASK_INSR() here (patch 3 already uses one right now).

Jan

On Thu, Oct 09, 2025 at 09:21:32AM +0200, Jan Beulich wrote:
> On 08.10.2025 18:23, Roger Pau Monné wrote:
> > On Wed, Oct 08, 2025 at 02:08:48PM +0200, Jan Beulich wrote:
> >> @@ -1439,7 +1438,7 @@ static void vlapic_do_init(struct vlapic
> >>      if ( !has_vlapic(vlapic_vcpu(vlapic)->domain) )
> >>          return;
> >>  
> >> -    vlapic_set_reg(vlapic, APIC_LVR, VLAPIC_VERSION);
> >> +    vlapic_set_reg(vlapic, APIC_LVR, 0x00050014);
> > 
> > (Maybe I'm getting ahead of patch 3, as I haven't looked yet)
> > 
> > Don't we want to use some kind of macros to build this in a more
> > friendly way?
> > 
> > We could have a pair of SET_APIC_{VERSION,MAXLVT}()?
> 
> With what patch 3 does to apicdef.h, I was rather wondering whether to simply
> use two MASK_INSR() here (patch 3 already uses one right now).

That would be fine for me.

Thanks, Roger.