Re: [PATCH v2 3/9] x86/svm: move declarations used only by svm code from svm.h to private header

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/02/2023 12:00 pm, Xenia Ragiadakou wrote:
> Create a new private header in arch/x86/hvm/svm called svm.h and move there
> all definitions and declarations that are used solely by svm code.
>
> The function svm_invlpga() stays in arch/x86/hvm/svm/svm.h because it is used
> by arch/x86/hvm/svm/asid.h.

I'm reasonably sure that all headers in arch/x86/hvm/svm/ other than
svm.h can move to being private easily.

>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> ---
>
> Changes in v2:
>  - new patch, the creation of a private header was suggested by Andrew and Jan
>
> I have not added #ifndef guards as it is a private and it should not be
> included by other headers. However, this is considered a MISRA-C violation
> ... I 'm not sure what to do.

Always have guards.  Firstly because that is the decision taken by the
MISRA working group.

But more importantly, because life is too short to deal with the
shooting yourself in the foot which will occur from trying to take
shortcuts like these.


> diff --git a/xen/arch/x86/hvm/svm/svm.h b/xen/arch/x86/hvm/svm/svm.h
> new file mode 100644
> index 0000000000..b2ec293078
> --- /dev/null
> +++ b/xen/arch/x86/hvm/svm/svm.h
> @@ -0,0 +1,40 @@
> +#include <xen/types.h>

Elsewhere, we're retrofitting SPDX tags to all source files, but we're
already putting tags in new source files.

This one needs to be /* SPDX-License-Identifier: GPL-2.0 */ I think.

> +
> +static inline void svm_vmload_pa(paddr_t vmcb)
> +{
> +    asm volatile (
> +        ".byte 0x0f,0x01,0xda" /* vmload */
> +        : : "a" (vmcb) : "memory" );
> +}
> +
> +static inline void svm_vmsave_pa(paddr_t vmcb)
> +{
> +    asm volatile (
> +        ".byte 0x0f,0x01,0xdb" /* vmsave */
> +        : : "a" (vmcb) : "memory" );
> +}
> +
> +struct cpu_user_regs;

Looking at this, you could fold patch 1 into this patch and reduce the
net churn.  It would be fine to say "delete used forward declaration" in
the commit message, seeing as you're deleting that block of code anyway
from svm.h

If you want to do this, then I'll skip committing patch 1.

~Andrew