On 16/05/2024 16:37, Alejandro Vallejo wrote:
> On 30/04/2024 15:42, Anthony PERARD wrote:
>> On Wed, Feb 07, 2024 at 05:39:57PM +0000, Alejandro Vallejo wrote:
>>> diff --git a/tools/libs/guest/xg_cpuid_x86.c
>>> b/tools/libs/guest/xg_cpuid_x86.c
>>> index 5699a26b946e..cee0be80ba5b 100644
>>> --- a/tools/libs/guest/xg_cpuid_x86.c
>>> +++ b/tools/libs/guest/xg_cpuid_x86.c
>>> @@ -772,49 +616,45 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t
>>> domid, bool restore,
>>> * apic_id_size values greater than 7. Limit the value to
>>> * 7 for now.
>>> */
>>> - if ( p->policy.extd.nc < 0x7f )
>>> + if ( cur->policy.extd.nc < 0x7f )
>>> {
>>> - if ( p->policy.extd.apic_id_size != 0 &&
>>> p->policy.extd.apic_id_size < 0x7 )
>>> - p->policy.extd.apic_id_size++;
>>> + if ( cur->policy.extd.apic_id_size != 0 &&
>>> cur->policy.extd.apic_id_size < 0x7 )
>>> + cur->policy.extd.apic_id_size++;
>>>
>>> - p->policy.extd.nc = (p->policy.extd.nc << 1) | 1;
>>> + cur->policy.extd.nc = (cur->policy.extd.nc << 1) | 1;
>>> }
>>> break;
>>> }
>>> }
>>>
>>> - nr_leaves = ARRAY_SIZE(p->leaves);
>>> - rc = x86_cpuid_copy_to_buffer(&p->policy, p->leaves, &nr_leaves);
>>> - if ( rc )
>>> + if ( xend || msr )
>>> {
>>> - ERROR("Failed to serialise CPUID (%d = %s)", -rc, strerror(-rc));
>>> - goto out;
>>> + // The overrides are over the serialised form of the policy
>>
>> Comments should use /* */
>
> Ugh, yes.
>
>>
>>> + if ( (rc = xc_cpu_policy_serialise(xch, cur)) )
>>> + goto out;
>>> +
>>> + if ( (rc = xc_cpuid_xend_policy(xch, domid, xend, host, def, cur))
>>> )
>>> + goto out;
>>> + if ( (rc = xc_msr_policy(xch, domid, msr, host, def, cur)) )
>>> + goto out;
>>
>> What if `xend` is set, but `msr` isn't? Looks like there's going to be a
>> segv in xc_msr_policy() because it doesn't check that `msr` is actually
>> set.
>>
>>
>> Thanks,
>>
>
> OOPS! Yes, msrs was meant to have the same check I added for
> xc_cpuid_xend_policy. Will do.
>
> Cheers,
> Alejandro
Ugh answered to the old email. pinging to the xenproject one now.
Cheers,
Alejandro
