RE: [RFC PATCH 11/18] virt/mshv: set up synic pages for intercept messages

[Michael Kelley <mikelley@microsoft.com>]

From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Thursday, March 11, 2021 11:38 AM
> 
> On 2/8/2021 11:47 AM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@linux.microsoft.com> Sent: Friday, November
> 20, 2020 4:31 PM
> >>
> >> Same idea as synic setup in drivers/hv/hv.c:hv_synic_enable_regs()
> >> and hv_synic_disable_regs().
> >> Setting up synic registers in both vmbus driver and mshv would clobber
> >> them, but the vmbus driver will not run in the root partition, so this
> >> is safe.
> >>
> >> Co-developed-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
> >> Signed-off-by: Lillian Grassin-Drake <ligrassi@microsoft.com>
> >> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> >> ---
> >>  arch/x86/include/asm/hyperv-tlfs.h      |  29 ---
> >>  arch/x86/include/uapi/asm/hyperv-tlfs.h | 264 ++++++++++++++++++++++++
> >>  include/asm-generic/hyperv-tlfs.h       |  46 +----
> >>  include/linux/mshv.h                    |   1 +
> >>  include/uapi/asm-generic/hyperv-tlfs.h  |  43 ++++
> >>  virt/mshv/mshv_main.c                   |  98 ++++++++-
> >>  6 files changed, 404 insertions(+), 77 deletions(-)
> >>
> >> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> >> index 4cd44ae9bffb..c34a6bb4f457 100644
> >> --- a/arch/x86/include/asm/hyperv-tlfs.h
> >> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> >> @@ -267,35 +267,6 @@ struct hv_tsc_emulation_status {
> >>  #define HV_X64_MSR_TSC_REFERENCE_ENABLE		0x00000001
> >>  #define HV_X64_MSR_TSC_REFERENCE_ADDRESS_SHIFT	12
> >>
> >> -
> >> -/* Define hypervisor message types. */
> >> -enum hv_message_type {
> >> -	HVMSG_NONE			= 0x00000000,
> >> -
> >> -	/* Memory access messages. */
> >> -	HVMSG_UNMAPPED_GPA		= 0x80000000,
> >> -	HVMSG_GPA_INTERCEPT		= 0x80000001,
> >> -
> >> -	/* Timer notification messages. */
> >> -	HVMSG_TIMER_EXPIRED		= 0x80000010,
> >> -
> >> -	/* Error messages. */
> >> -	HVMSG_INVALID_VP_REGISTER_VALUE	= 0x80000020,
> >> -	HVMSG_UNRECOVERABLE_EXCEPTION	= 0x80000021,
> >> -	HVMSG_UNSUPPORTED_FEATURE	= 0x80000022,
> >> -
> >> -	/* Trace buffer complete messages. */
> >> -	HVMSG_EVENTLOG_BUFFERCOMPLETE	= 0x80000040,
> >> -
> >> -	/* Platform-specific processor intercept messages. */
> >> -	HVMSG_X64_IOPORT_INTERCEPT	= 0x80010000,
> >> -	HVMSG_X64_MSR_INTERCEPT		= 0x80010001,
> >> -	HVMSG_X64_CPUID_INTERCEPT	= 0x80010002,
> >> -	HVMSG_X64_EXCEPTION_INTERCEPT	= 0x80010003,
> >> -	HVMSG_X64_APIC_EOI		= 0x80010004,
> >> -	HVMSG_X64_LEGACY_FP_ERROR	= 0x80010005
> >> -};
> >> -
> >>  struct hv_nested_enlightenments_control {
> >>  	struct {
> >>  		__u32 directhypercall:1;
> >> diff --git a/arch/x86/include/uapi/asm/hyperv-tlfs.h
> b/arch/x86/include/uapi/asm/hyperv-
> >> tlfs.h
> >> index 2ff655962738..c6a27053f791 100644
> >> --- a/arch/x86/include/uapi/asm/hyperv-tlfs.h
> >> +++ b/arch/x86/include/uapi/asm/hyperv-tlfs.h
> >> @@ -722,4 +722,268 @@ union hv_register_value {
> >>  		pending_virtualization_fault_event;
> >>  };
> >>
> >> +/* Define hypervisor message types. */
> >> +enum hv_message_type {
> >> +	HVMSG_NONE				= 0x00000000,
> >> +
> >> +	/* Memory access messages. */
> >> +	HVMSG_UNMAPPED_GPA			= 0x80000000,
> >> +	HVMSG_GPA_INTERCEPT			= 0x80000001,
> >> +
> >> +	/* Timer notification messages. */
> >> +	HVMSG_TIMER_EXPIRED			= 0x80000010,
> >> +
> >> +	/* Error messages. */
> >> +	HVMSG_INVALID_VP_REGISTER_VALUE		= 0x80000020,
> >> +	HVMSG_UNRECOVERABLE_EXCEPTION		= 0x80000021,
> >> +	HVMSG_UNSUPPORTED_FEATURE		= 0x80000022,
> >> +
> >> +	/* Trace buffer complete messages. */
> >> +	HVMSG_EVENTLOG_BUFFERCOMPLETE		= 0x80000040,
> >> +
> >> +	/* Platform-specific processor intercept messages. */
> >> +	HVMSG_X64_IO_PORT_INTERCEPT		= 0x80010000,
> >> +	HVMSG_X64_MSR_INTERCEPT			= 0x80010001,
> >> +	HVMSG_X64_CPUID_INTERCEPT		= 0x80010002,
> >> +	HVMSG_X64_EXCEPTION_INTERCEPT		= 0x80010003,
> >> +	HVMSG_X64_APIC_EOI			= 0x80010004,
> >> +	HVMSG_X64_LEGACY_FP_ERROR		= 0x80010005,
> >> +	HVMSG_X64_IOMMU_PRQ			= 0x80010006,
> >> +	HVMSG_X64_HALT				= 0x80010007,
> >> +	HVMSG_X64_INTERRUPTION_DELIVERABLE	= 0x80010008,
> >> +	HVMSG_X64_SIPI_INTERCEPT		= 0x80010009,
> >> +};
> >
> > I have a separate patch series that moves this enum to the
> > asm-generic portion of hyperv-tlfs.h because there's not a good way
> > to separate the arch neutral from arch dependent values.
> >
> 
> Ok, but it should also be changed to #define instead of an enum, right?
> I will do that in this patch.
> This requires a couple of changes in other files in drivers/hv
> where this enum is used.

Because of the other uses of the enum in places that don't depend
on exact structure layouts, I left it as an enum when I moved it.
When one of the enum values is passed to Hyper-V, the enum
is assigned to a u32 field, which I think is acceptable.  You could
do the same with the other enums your already have -- keep the
constant definitions as members of an enum, but assign to a u32
field in the structures that get passed to Hyper-V.  There may
actually be some benefit in that approach, particularly if the enum
is passed as an individual argument into some function(s). 

Others may have an opinion on this approach .....

Michael