On 17.03.2025 08:19, Sergiy Kibrik wrote:
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -348,6 +348,11 @@ config HYPERV_GUEST
>
> endif
>
> +config HVM_VIRIDIAN
> + bool "Viridian enlightenments support" if EXPERT
> + depends on HVM
> + default y
Imo the prompt wants to include "guest", somewhat along the lines of what
Alejandro has also said.
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -695,9 +695,12 @@ int hvm_domain_initialise(struct domain *d,
> if ( hvm_tsc_scaling_supported )
> d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>
> - rc = viridian_domain_init(d);
> - if ( rc )
> - goto fail2;
> + if ( is_viridian_domain(d) )
> + {
> + rc = viridian_domain_init(d);
> + if ( rc )
> + goto fail2;
> + }
>
> rc = alternative_call(hvm_funcs.domain_initialise, d);
> if ( rc != 0 )
> @@ -733,7 +736,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
> if ( hvm_funcs.nhvm_domain_relinquish_resources )
> alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
>
> - viridian_domain_deinit(d);
> + if ( is_viridian_domain(d) )
> + viridian_domain_deinit(d);
>
> ioreq_server_destroy_all(d);
>
> @@ -1637,9 +1641,12 @@ int hvm_vcpu_initialise(struct vcpu *v)
> && (rc = nestedhvm_vcpu_initialise(v)) < 0 ) /* teardown:
> nestedhvm_vcpu_destroy */
> goto fail5;
>
> - rc = viridian_vcpu_init(v);
> - if ( rc )
> - goto fail6;
> + if ( is_viridian_domain(v->domain) )
Like you do further up, please also use "d" here and ...
> @@ -1669,13 +1676,15 @@ int hvm_vcpu_initialise(struct vcpu *v)
> fail2:
> hvm_vcpu_cacheattr_destroy(v);
> fail1:
> - viridian_vcpu_deinit(v);
> + if ( is_viridian_domain(v->domain) )
... here.
> --- a/xen/arch/x86/hvm/vlapic.c
> +++ b/xen/arch/x86/hvm/vlapic.c
> @@ -426,7 +426,8 @@ void vlapic_EOI_set(struct vlapic *vlapic)
> * priority vector and then recurse to handle the lower priority
> * vector.
> */
> - bool missed_eoi = viridian_apic_assist_completed(v);
> + bool missed_eoi = has_viridian_apic_assist(v->domain) ?
> + viridian_apic_assist_completed(v) : false;
bool missed_eoi = has_viridian_apic_assist(v->domain) &&
viridian_apic_assist_completed(v);
?
> --- a/xen/arch/x86/include/asm/hvm/domain.h
> +++ b/xen/arch/x86/include/asm/hvm/domain.h
> @@ -110,9 +110,9 @@ struct hvm_domain {
>
> /* hypervisor intercepted msix table */
> struct list_head msixtbl_list;
> -
> +#ifdef CONFIG_HVM_VIRIDIAN
> struct viridian_domain *viridian;
> -
> +#endif
> /*
> * TSC value that VCPUs use to calculate their tsc_offset value.
> * Used during initialization and save/restore.
Why would the blank lines need to go away?
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -507,7 +507,8 @@ hvm_get_cpl(struct vcpu *v)
> (has_hvm_params(d) ? (d)->arch.hvm.params[HVM_PARAM_VIRIDIAN] : 0)
>
> #define is_viridian_domain(d) \
> - (is_hvm_domain(d) && (viridian_feature_mask(d) & HVMPV_base_freq))
> + (is_hvm_domain(d) && IS_ENABLED(CONFIG_HVM_VIRIDIAN) && \
> + (viridian_feature_mask(d) & HVMPV_base_freq))
May I suggest to put IS_ENABLED() first? And to adjust (reduce) indentation
on the 2nd line?
> --- a/xen/arch/x86/include/asm/hvm/vcpu.h
> +++ b/xen/arch/x86/include/asm/hvm/vcpu.h
> @@ -171,8 +171,9 @@ struct hvm_vcpu {
>
> /* Pending hw/sw interrupt (.vector = -1 means nothing pending). */
> struct x86_event inject_event;
> -
> +#ifdef CONFIG_HVM_VIRIDIAN
> struct viridian_vcpu *viridian;
> +#endif
> };
Again, not need for the blank line to go away.
Jan
