On 23/05/2024 15:50, Roger Pau Monné wrote:
> On Wed, May 08, 2024 at 01:39:22PM +0100, Alejandro Vallejo wrote:
>> While at it, add a check for the reserved field in the hidden save area.
>>
>> Signed-off-by: Alejandro Vallejo <alejandro.vall...@cloud.com>
>> ---
>> v2:
>> * New patch. Addresses the missing check for rsvd_zero in v1.
>
> Oh, it would be better if this was done at the time when rsvd_zero is
> introduced. I think this should be moved ahead of the series, so that
> the patch that introduces rsvd_zero can add the check in
> lapic_check_hidden().
I'll give that a whirl.
>
>> ---
>> xen/arch/x86/hvm/vlapic.c | 41 ++++++++++++++++++++++++++++-----------
>> 1 file changed, 30 insertions(+), 11 deletions(-)
>>
>> diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c
>> index 8a244100009c..2f06bff1b2cc 100644
>> --- a/xen/arch/x86/hvm/vlapic.c
>> +++ b/xen/arch/x86/hvm/vlapic.c
>> @@ -1573,35 +1573,54 @@ static void lapic_load_fixup(struct vlapic *vlapic)
>> v, vlapic->loaded.id, vlapic->loaded.ldr, good_ldr);
>> }
>>
>> -static int cf_check lapic_load_hidden(struct domain *d,
>> hvm_domain_context_t *h)
>> +static int cf_check lapic_check_hidden(const struct domain *d,
>> + hvm_domain_context_t *h)
>> {
>> unsigned int vcpuid = hvm_load_instance(h);
>> - struct vcpu *v;
>> - struct vlapic *s;
>> + struct hvm_hw_lapic s;
>>
>> if ( !has_vlapic(d) )
>> return -ENODEV;
>>
>> /* Which vlapic to load? */
>> - if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
>> + if ( vcpuid >= d->max_vcpus || d->vcpu[vcpuid] == NULL )
>> {
>> dprintk(XENLOG_G_ERR, "HVM restore: dom%d has no apic%u\n",
>> d->domain_id, vcpuid);
>> return -EINVAL;
>> }
>> - s = vcpu_vlapic(v);
>>
>> - if ( hvm_load_entry_zeroextend(LAPIC, h, &s->hw) != 0 )
>> + if ( hvm_load_entry_zeroextend(LAPIC, h, &s) )
>
> Can't you use hvm_get_entry() to perform the sanity checks:
>
> const struct hvm_hw_lapic *s = hvm_get_entry(LAPIC, h);
>
> Thanks, Roger.
I don't think I can. Because the last field (rsvd_zero) might or might
not be there, so it needs to be zero-extended. Unless I misunderstood
what hvm_get_entry() is meant to do. It seems to check for exact sizes.
Cheers,
Alejandro
