From: Tianyu Lan <[email protected]> Sent: Saturday, July 26, 2025 6:43 AM >
Nit: The patch "Subject:" prefix here should be "Drivers: hv:" with no slash, as it was in v3 of the patch. That's admittedly not consistent with "x86/hyperv:" that is used for the other patches in this series, but it is consistent with historical practice for the files in the drivers/hv folder. You have to look at past commits for a particular file to see what the typical prefix is. Michael > When Secure AVIC is enabled, VMBus driver should > call x2apic Secure AVIC interface to allow Hyper-V > to inject VMBus message interrupt. > > Signed-off-by: Tianyu Lan <[email protected]> > --- > Change since RFC V3: > - Disable VMBus Message interrupt via hv_enable_ > coco_interrupt() in the hv_synic_disable_regs(). > --- > arch/x86/hyperv/hv_apic.c | 5 +++++ > drivers/hv/hv.c | 2 ++ > drivers/hv/hv_common.c | 5 +++++ > include/asm-generic/mshyperv.h | 1 + > 4 files changed, 13 insertions(+) > > diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c > index e669053b637d..a8de503def37 100644 > --- a/arch/x86/hyperv/hv_apic.c > +++ b/arch/x86/hyperv/hv_apic.c > @@ -53,6 +53,11 @@ static void hv_apic_icr_write(u32 low, u32 id) > wrmsrq(HV_X64_MSR_ICR, reg_val); > } > > +void hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool > set) > +{ > + apic_update_vector(cpu, vector, set); > +} > + > static u32 hv_apic_read(u32 reg) > { > u32 reg_val, hi; > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c > index 308c8f279df8..aa384dbf38ac 100644 > --- a/drivers/hv/hv.c > +++ b/drivers/hv/hv.c > @@ -310,6 +310,7 @@ void hv_synic_enable_regs(unsigned int cpu) > if (vmbus_irq != -1) > enable_percpu_irq(vmbus_irq, 0); > shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT); > + hv_enable_coco_interrupt(cpu, vmbus_interrupt, true); > > shared_sint.vector = vmbus_interrupt; > shared_sint.masked = false; Something I just noticed. The existing code in hv_synic_enable_regs() is reading the SINT MSR, updating some values, and then writing back the SINT MSR. Those steps act as a unit to update the MSR. You've added the call to hv_enable_coco_interrupts() in the middle of that unit, which implies there might be a reason for it. If there's not a reason, I would expect the call to hv_enable_coco_interrupt() to be before the unit, not in the middle of it. > @@ -342,6 +343,7 @@ void hv_synic_disable_regs(unsigned int cpu) > union hv_synic_scontrol sctrl; > > shared_sint.as_uint64 = hv_get_msr(HV_MSR_SINT0 + VMBUS_MESSAGE_SINT); > + hv_enable_coco_interrupt(cpu, vmbus_interrupt, false); Same here with the hv_enable_coco_interrupt() call in the middle of the unit that is updating the SINT MSR. In the disable path, I would have expected hv_enable_coco_interrupt() to be *after* the unit so that disable operations are in reverse order of the corresponding enable operation. > > shared_sint.masked = 1; > > diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c > index 49898d10faff..0f024ab3d360 100644 > --- a/drivers/hv/hv_common.c > +++ b/drivers/hv/hv_common.c > @@ -716,6 +716,11 @@ u64 __weak hv_tdx_hypercall(u64 control, u64 param1, u64 > param2) > } > EXPORT_SYMBOL_GPL(hv_tdx_hypercall); > > +void __weak hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, > bool set) > +{ > +} > +EXPORT_SYMBOL_GPL(hv_enable_coco_interrupt); > + > void hv_identify_partition_type(void) > { > /* Assume guest role */ > diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h > index a729b77983fa..7907c9878369 100644 > --- a/include/asm-generic/mshyperv.h > +++ b/include/asm-generic/mshyperv.h > @@ -333,6 +333,7 @@ bool hv_is_isolation_supported(void); > bool hv_isolation_type_snp(void); > u64 hv_ghcb_hypercall(u64 control, void *input, void *output, u32 > input_size); > u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2); > +void hv_enable_coco_interrupt(unsigned int cpu, unsigned int vector, bool > set); > void hyperv_cleanup(void); > bool hv_query_ext_cap(u64 cap_query); > void hv_setup_dma_ops(struct device *dev, bool coherent); > -- > 2.25.1 >
