RE: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-03 Thread Dexuan Cui
> From: David Woodhouse 
> Sent: Tuesday, November 3, 2020 12:03 AM
> > +/*
> > + * If ms_hyperv_msi_ext_dest_id() returns true,
> > hyperv_prepare_irq_remapping()
> > + * returns -ENODEV and the Hyper-V IOMMU driver is not used; instead, the
> > + * generic support of the 15-bit APIC ID is used: see
> > __irq_msi_compose_msg().
> > + *
> > + * Note: For a VM on Hyper-V, no emulated legacy device supports PCI
> MSI/MSI-X,
> > + * and PCI MSI/MSI-X only come from the assigned physical PCIe device, and
> the
> > + * PCI MSI/MSI-X interrupts are handled by the pci-hyperv driver. Here
> despite
> > + * the word "msi" in the name "msi_ext_dest_id", actually the callback only
> > + * affects how IOAPIC interrupts are routed.
> > + */
> 
> I named it like that on purpose to make the point that the I/OAPIC is
> just a device for turning line interrupts into MSIs. Some VMMs, just
> like real hardware, really do implement their I/OAPIC emulation that
> way. It makes a lot of sense to do so if you support interrupt
> remapping.

I totally agree.
 
> FWIW I might have phrased your last paragraph in that comment as
> 
>   Note: for a VM on Hyper-V, the I/OAPIC is the only device which
>   (logically) generates MSIs directly to the system APIC irq domain.
>   There is no HPET, and PCI MSI/MSI-X interrupts are remapped by the
>   pci-hyperv host bridge.

I agree. This version is much better.
 
> But don't bother to change it; I think I've made my point quite well
> enough with https://git.kernel.org/tip/tip/c/5d5a97133 :)
> 
> --
> dwmw2

Hi David,
This patch has been in the x86/apic branch (with a line missing in the commit
log). If possible, I hope tglx can help make this change you suggested, and add
the missing line in the commit log. :-)

Thanks,
-- Dexuan


RE: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-03 Thread Dexuan Cui
> From: Dexuan Cui 
> Sent: Monday, November 2, 2020 5:12 PM
> 
> ...

Hi tglx,
Now this patch is in the x86/apic branch, which is great! Thanks for the 
quick action! But the third line of the below paragraph of the commit log
is missing... Sorry I just realized I should have not prefixed that line with 
the
">255 APIC IDs" -- it looks a line is ignored if it starts with 2 chars of 
">>". :-(

> On an old Hyper-V host that doesn't support the Extended Dest ID, nothing
> changes with this commit: Linux VM is still able to bring up the CPUs with
> >255 APIC IDs with the help of hyperv-iommu.c, but IOAPIC interrupts still
> can not go to such CPUs, and the kdump kernel still can not work properly
> on such CPUs.


Re: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-03 Thread David Woodhouse
On Mon, 2020-11-02 at 17:11 -0800, Dexuan Cui wrote:
> When a Linux VM runs on Hyper-V, if the VM has CPUs with >255 APIC IDs,
> the CPUs can't be the destination of IOAPIC interrupts, because the
> IOAPIC RTE's Dest Field has only 8 bits. Currently the hackery driver
> drivers/iommu/hyperv-iommu.c is used to ensure IOAPIC interrupts are
> only routed to CPUs that don't have >255 APIC IDs. However, there is
> an issue with kdump, because the kdump kernel can run on any CPU, and
> hence IOAPIC interrupts can't work if the kdump kernel run on a CPU
> with a >255 APIC ID.
> 
> The kdump issue can be fixed by the Extended Dest ID, which is introduced
> recently by David Woodhouse (for IOAPIC, see the field virt_destid_8_14 in
> struct IO_APIC_route_entry). Of course, the Extended Dest ID needs the
> support of the underlying hypervisor. The latest Hyper-V has added the
> support recently: with this commit, on such a Hyper-V host, Linux VM
> does not use hyperv-iommu.c because hyperv_prepare_irq_remapping()
> returns -ENODEV; instead, Linux kernel's generic support of Extended Dest
> ID from David is used, meaning that Linux VM is able to support up to
> 32K CPUs, and IOAPIC interrupts can be routed to all the CPUs.
> 
> On an old Hyper-V host that doesn't support the Extended Dest ID, nothing
> changes with this commit: Linux VM is still able to bring up the CPUs with
> > 255 APIC IDs with the help of hyperv-iommu.c, but IOAPIC interrupts still
> 
> can not go to such CPUs, and the kdump kernel still can not work properly
> on such CPUs.
> 
> Signed-off-by: Dexuan Cui 

Acked-by: David Woodhouse 

> +/*
> + * If ms_hyperv_msi_ext_dest_id() returns true, 
> hyperv_prepare_irq_remapping()
> + * returns -ENODEV and the Hyper-V IOMMU driver is not used; instead, the
> + * generic support of the 15-bit APIC ID is used: see 
> __irq_msi_compose_msg().
> + *
> + * Note: For a VM on Hyper-V, no emulated legacy device supports PCI 
> MSI/MSI-X,
> + * and PCI MSI/MSI-X only come from the assigned physical PCIe device, and 
> the
> + * PCI MSI/MSI-X interrupts are handled by the pci-hyperv driver. Here 
> despite
> + * the word "msi" in the name "msi_ext_dest_id", actually the callback only
> + * affects how IOAPIC interrupts are routed.
> + */

I named it like that on purpose to make the point that the I/OAPIC is
just a device for turning line interrupts into MSIs. Some VMMs, just
like real hardware, really do implement their I/OAPIC emulation that
way. It makes a lot of sense to do so if you support interrupt
remapping.

FWIW I might have phrased your last paragraph in that comment as

  Note: for a VM on Hyper-V, the I/OAPIC is the only device which
  (logically) generates MSIs directly to the system APIC irq domain.
  There is no HPET, and PCI MSI/MSI-X interrupts are remapped by the
  pci-hyperv host bridge.

But don't bother to change it; I think I've made my point quite well
enough with https://git.kernel.org/tip/tip/c/5d5a97133 :)

-- 
dwmw2




smime.p7s
Description: S/MIME cryptographic signature


Re: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-02 Thread kernel test robot
Hi Dexuan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on tip/x86/core]
[also build test ERROR on tip/master v5.10-rc2 next-20201102]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Dexuan-Cui/x86-hyperv-Enable-15-bit-APIC-ID-if-the-hypervisor-supports-it/20201103-091414
base:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 
238c91115cd05c71447ea071624a4c9fe661f970
config: i386-randconfig-r012-20201103 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# 
https://github.com/0day-ci/linux/commit/c4037b8c4cd61f970749c6685a3df5a1376193d2
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review 
Dexuan-Cui/x86-hyperv-Enable-15-bit-APIC-ID-if-the-hypervisor-supports-it/20201103-091414
git checkout c4037b8c4cd61f970749c6685a3df5a1376193d2
# save the attached .config to linux build tree
make W=1 ARCH=i386 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All error/warnings (new ones prefixed by >>):

>> arch/x86/kernel/cpu/mshyperv.c:397:8: error: 'struct x86_hyper_init' has no 
>> member named 'msi_ext_dest_id'
 397 |  .init.msi_ext_dest_id = ms_hyperv_msi_ext_dest_id,
 |^~~
>> arch/x86/kernel/cpu/mshyperv.c:397:26: error: initialization of 'void 
>> (*)(void)' from incompatible pointer type 'bool (*)(void)' {aka '_Bool 
>> (*)(void)'} [-Werror=incompatible-pointer-types]
 397 |  .init.msi_ext_dest_id = ms_hyperv_msi_ext_dest_id,
 |  ^
   arch/x86/kernel/cpu/mshyperv.c:397:26: note: (near initialization for 
'x86_hyper_ms_hyperv.init.init_platform')
>> arch/x86/kernel/cpu/mshyperv.c:398:24: warning: initialized field 
>> overwritten [-Woverride-init]
 398 |  .init.init_platform = ms_hyperv_init_platform,
 |^~~
   arch/x86/kernel/cpu/mshyperv.c:398:24: note: (near initialization for 
'x86_hyper_ms_hyperv.init.init_platform')
   cc1: some warnings being treated as errors

vim +397 arch/x86/kernel/cpu/mshyperv.c

   391  
   392  const __initconst struct hypervisor_x86 x86_hyper_ms_hyperv = {
   393  .name   = "Microsoft Hyper-V",
   394  .detect = ms_hyperv_platform,
   395  .type   = X86_HYPER_MS_HYPERV,
   396  .init.x2apic_available  = ms_hyperv_x2apic_available,
 > 397  .init.msi_ext_dest_id   = ms_hyperv_msi_ext_dest_id,
 > 398  .init.init_platform = ms_hyperv_init_platform,

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org


.config.gz
Description: application/gzip


RE: [PATCH] x86/hyperv: Enable 15-bit APIC ID if the hypervisor supports it

2020-11-02 Thread Dexuan Cui
> From: Dexuan Cui 
> Sent: Monday, November 2, 2020 5:12 PM

Sorry I forgot to mention that this patch is based on tip.git's x86/apic branch.

Thanks,
-- Dexuan