On 04.08.2025 17:41, Jan Beulich wrote:
> While the logic there covers asymmetric cases, the resulting log
> messages would likely raise more confusion than clarify anything. Split
> the BSP action from the AP one, indicating the odd CPU in the AP log
> message, thus avoiding the impression that global state would have
> changed.
>
> While there also
> - move g_disabled into .data.ro_after_init; only the BSP will ever write
> to it,
> - make the function's parameter unsigned; no negative values may be
> passed in. Also reflect this in svm_asid_init().
>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
Sorry, I meant to Cc: you on the submission, as it was the reviewing of
your copied code which made me spot the shortcomings (which, as indicated,
I think it would be nice if you avoided from the very start).
Jan
> --- a/xen/arch/x86/hvm/asid.c
> +++ b/xen/arch/x86/hvm/asid.c
> @@ -48,20 +48,22 @@ struct hvm_asid_data {
>
> static DEFINE_PER_CPU(struct hvm_asid_data, hvm_asid_data);
>
> -void hvm_asid_init(int nasids)
> +void hvm_asid_init(unsigned int nasids)
> {
> - static int8_t g_disabled = -1;
> + static int8_t __ro_after_init g_disabled = -1;
> struct hvm_asid_data *data = &this_cpu(hvm_asid_data);
>
> data->max_asid = nasids - 1;
> data->disabled = !opt_asid_enabled || (nasids <= 1);
>
> - if ( g_disabled != data->disabled )
> + if ( g_disabled < 0 )
> {
> - printk("HVM: ASIDs %sabled.\n", data->disabled ? "dis" : "en");
> - if ( g_disabled < 0 )
> - g_disabled = data->disabled;
> + g_disabled = data->disabled;
> + printk("HVM: ASIDs %sabled\n", data->disabled ? "dis" : "en");
> }
> + else if ( g_disabled != data->disabled )
> + printk("HVM: CPU%u: ASIDs %sabled\n", smp_processor_id(),
> + data->disabled ? "dis" : "en");
>
> /* Zero indicates 'invalid generation', so we start the count at one. */
> data->core_asid_generation = 1;
> --- a/xen/arch/x86/hvm/svm/asid.c
> +++ b/xen/arch/x86/hvm/svm/asid.c
> @@ -12,7 +12,7 @@
>
> void svm_asid_init(const struct cpuinfo_x86 *c)
> {
> - int nasids = 0;
> + unsigned int nasids = 0;
>
> /* Check for erratum #170, and leave ASIDs disabled if it's present. */
> if ( !cpu_has_amd_erratum(c, AMD_ERRATUM_170) )
> --- a/xen/arch/x86/include/asm/hvm/asid.h
> +++ b/xen/arch/x86/include/asm/hvm/asid.h
> @@ -13,7 +13,7 @@ struct vcpu;
> struct hvm_vcpu_asid;
>
> /* Initialise ASID management for the current physical CPU. */
> -void hvm_asid_init(int nasids);
> +void hvm_asid_init(unsigned int nasids);
>
> /* Invalidate a particular ASID allocation: forces re-allocation. */
> void hvm_asid_flush_vcpu_asid(struct hvm_vcpu_asid *asid);
