Re: [PATCH 2/7] x86/apic: Add an SVSM APIC driver

[Tom Lendacky <thomas.lendacky@amd.com>]

On 8/24/26 21:06, Melody Wang wrote:
> Hi Tom,
> 
> On 8/21/26 7:40 AM, Tom Lendacky wrote:
> 
>> If that is the case, then the fix would be to move the setting of the CAA
>> into the svsm_perform_call_protocol() function and just make that function
>> external, which will eliminate the need for svsm_do_call().
>>
>> You would be a pre-patch then for fixing all the callers of
>> svsm_perform_call_protocol().
> 
> This is the code, do you think this is what is in your mind?

Yes, just some comments below.

> 
> diff --git a/arch/x86/coco/sev/svsm.c b/arch/x86/coco/sev/svsm.c
> index e2d3bc3c26eb..f036a85a043b 100644
> --- a/arch/x86/coco/sev/svsm.c
> +++ b/arch/x86/coco/sev/svsm.c
> @@ -74,6 +74,9 @@ int svsm_perform_call_protocol(struct svsm_call *call)
> 
>         flags = native_local_irq_save();
> 
> +       if (!call->caa)
> +               call->caa = svsm_get_caa();

I wouldn't make this conditional. Also a comment above it that indicates
the reason for setting it after native_local_irq_save() would be good to have.

> +
>         ghcb = __sev_get_ghcb(&state);
> 
>         do {
> @@ -87,6 +90,7 @@ int svsm_perform_call_protocol(struct svsm_call *call)
> 
>         return ret;
>  }
> +EXPORT_SYMBOL_GPL(svsm_perform_call_protocol);
> 
>  static u64 svsm_build_ca_from_pfn_range(u64 pfn, u64 pfn_end, bool action,
>                                         struct svsm_pvalidate_call *pc)
> @@ -321,7 +325,6 @@ int snp_svsm_vtpm_send_command(u8 *buffer)
>  {
>         struct svsm_call call = {};
> 
> -       call.caa = svsm_get_caa();
>         call.rax = SVSM_VTPM_CALL(SVSM_VTPM_CMD);
>         call.rcx = __pa(buffer);
> 
> @@ -345,7 +348,6 @@ bool snp_svsm_vtpm_probe(void)
>         if (!snp_vmpl)
>                 return false;
> 
> -       call.caa = svsm_get_caa();
>         call.rax = SVSM_VTPM_CALL(SVSM_VTPM_QUERY);
> 
>         if (svsm_perform_call_protocol(&call))
> @@ -354,9 +356,3 @@ bool snp_svsm_vtpm_probe(void)
>         /* Check platform commands contains TPM_SEND_COMMAND - platform
> command 8 */
>         return call.rcx_out & BIT_ULL(8);
>  }
> -
> -int svsm_do_call(struct svsm_call *call)
> -{
> -       call->caa = svsm_get_caa();
> -       return svsm_perform_call_protocol(call);
> -}

As a pre-patch, this wouldn't exist, so just take the same approach but
before any of your patches have been applied.

Thanks,
Tom

> diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
> index f958f78e1db8..9013b392ad40 100644
> --- a/arch/x86/include/asm/sev.h
> +++ b/arch/x86/include/asm/sev.h
> @@ -526,7 +526,6 @@ u64 sev_get_status(void);
>  void sev_show_status(void);
>  int prepare_pte_enc(struct pte_enc_desc *d);
>  void set_pte_enc_mask(pte_t *kpte, unsigned long pfn, pgprot_t new_prot);
> -int svsm_do_call(struct svsm_call *call);
>  void snp_kexec_finish(void);
>  void snp_kexec_begin(void);
> 
> @@ -536,7 +535,7 @@ void snp_msg_free(struct snp_msg_desc *mdesc);
>  int snp_send_guest_request(struct snp_msg_desc *mdesc, struct
> snp_guest_req *req);
> 
>  int snp_svsm_vtpm_send_command(u8 *buffer);
> -
> +int svsm_perform_call_protocol(struct svsm_call *call);
>  void __init snp_secure_tsc_prepare(void);
>  void __init snp_secure_tsc_init(void);
>  enum es_result savic_register_gpa(u64 gpa);
> @@ -619,7 +618,6 @@ static inline int rmpadjust(unsigned long vaddr, bool
> rmp_psize, unsigned long a
>  static inline void setup_ghcb(void) { }
>  static inline void __init
>  early_snp_set_memory_private(unsigned long vaddr, unsigned long paddr,
> unsigned long npages) { }
> -static inline int svsm_do_call(struct svsm_call *call) { return 0; }
>  static inline void __init
>  early_snp_set_memory_shared(unsigned long vaddr, unsigned long paddr,
> unsigned long npages) { }
>  static inline void snp_set_memory_shared(unsigned long vaddr, unsigned
> long npages) { }
> @@ -645,6 +643,7 @@ static inline void snp_msg_free(struct snp_msg_desc
> *mdesc) { }
>  static inline int snp_send_guest_request(struct snp_msg_desc *mdesc,
>                                          struct snp_guest_req *req)
> { return -ENODEV; }
>  static inline int snp_svsm_vtpm_send_command(u8 *buffer) { return -ENODEV; }
> +static inline int svsm_perform_call_protocol(struct svsm_call *call)
> { return -ENODEV; }
>  static inline void __init snp_secure_tsc_prepare(void) { }
>  static inline void __init snp_secure_tsc_init(void) { }
>  static inline void sev_evict_cache(void *va, int npages) {}
> 
> Thanks,
> Melody
> 
>>
>> Thanks,
>> Tom
>>
>>>
>>
>