Re: [PATCH] x86/hvm: Add APIC IDs to the per-vLAPIC save area

["Roger Pau Monné" <roger.pau@citrix.com>]

On Thu, Feb 27, 2025 at 02:12:58PM +0000, Alejandro Vallejo wrote:
> Hi,
> 
> On Wed Feb 26, 2025 at 5:33 PM GMT, Roger Pau Monné wrote:
> > On Wed, Feb 26, 2025 at 02:11:23PM +0100, Jan Beulich wrote:
> > > On 18.02.2025 15:22, Alejandro Vallejo wrote:
> > > > @@ -1621,6 +1624,14 @@ static int cf_check lapic_load_hidden(struct domain *d, hvm_domain_context_t *h)
> > > >          return -EINVAL;
> > > >      }
> > > >  
> > > > +    /*
> > > > +     * Xen 4.20 and earlier had no x2APIC ID in the migration stream and
> > > > +     * hard-coded "vcpu_id * 2". Default back to this if we have a
> > > > +     * zero-extended record.
> > > > +     */
> > > > +    if ( h->size <= offsetof(struct hvm_hw_lapic, x2apic_id) )
> > > > +        s->hw.x2apic_id = v->vcpu_id * 2;
> > > 
> > > While we better wouldn't get to see such input, it is in principle possible
> > > to have an input stream with, say, half the field. Imo the condition ought
> > > to be such that we'd make the adjustment when less than the full field is
> > > available.
> >
> > I would add an additional check to ensure _rsvd0 remains 0, to avoid
> > further additions from attempting to reuse that padding space.
> >
> > if ( s->hw._rsvd0 )
> >     return -EINVAL;
> 
> That's already on lapic_check_hidden(), so it's guaranteed to be zero. Unless
> you mean something else?

Oh, I've missed that - it's indeed fine.  I was missing the previous
chunk when replying here and forgot about it.

Thanks, Roger.