Re: [PATCH v7 5/5] Driver: VMBus: Add Devicetree support

Saurabh Singh Sengar
2023-03-13T13:37:33Z

On Mon, Mar 13, 2023 at 12:26:56PM +0000, Michael Kelley (LINUX) wrote:
> From: Saurabh Singh Sengar <ssengar@linux.microsoft.com> Sent: Sunday, March 12, 2023 11:16 PM
> >
> > On Mon, Mar 13, 2023 at 02:33:53AM +0000, Michael Kelley (LINUX) wrote:
> > > From: Saurabh Sengar <ssengar@linux.microsoft.com> Sent: Thursday, February 23,
> > 2023 3:29 AM
> > > >
> > > > Update the driver to support Devicetree boot as well along with ACPI.
> > > > At present the Devicetree parsing only provides the mmio region info
> > > > and is not the exact copy of ACPI parsing. This is sufficient to cater
> > > > all the current Devicetree usecases for VMBus.
> > > >
> > > > Currently Devicetree is supported only for x86 systems.
> > > >
> > > > Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> > > > ---
> > > > [V7]
> > > > - Use cpu_addr instead of bus_addr
> > > >
> > > > drivers/hv/Kconfig | 6 +++--
> > > > drivers/hv/vmbus_drv.c | 57 ++++++++++++++++++++++++++++++++++++++++--
> > > > 2 files changed, 59 insertions(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/hv/Kconfig b/drivers/hv/Kconfig
> > > > index 0747a8f1fcee..1a55bf32d195 100644
> > > > --- a/drivers/hv/Kconfig
> > > > +++ b/drivers/hv/Kconfig
> > > > @@ -4,11 +4,13 @@ menu "Microsoft Hyper-V guest support"
> > > >
> > > > config HYPERV
> > > > tristate "Microsoft Hyper-V client drivers"
> > > > - depends on ACPI && ((X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
> > > > - || (ARM64 && !CPU_BIG_ENDIAN))
> > > > + depends on (X86 && X86_LOCAL_APIC && HYPERVISOR_GUEST) \
> > > > + || (ACPI && ARM64 && !CPU_BIG_ENDIAN)
> > > > select PARAVIRT
> > > > select X86_HV_CALLBACK_VECTOR if X86
> > > > select VMAP_PFN
> > > > + select OF if !ACPI
> > > > + select OF_EARLY_FLATTREE if !ACPI
> > > > help
> > > > Select this option to run Linux as a Hyper-V client operating
> > > > system.
> > >
> > > One further thing occurred to me. OF_EARLY_FLATTREE really depends
> > > on OF instead of ACPI. The ACPI dependency is indirect through OF. So
> > > I'd suggest doing
> > >
> > > select OF_EARLY_FLATTRE if OF
> > >
> > > to express the direct dependency.
> >
> > As you pointed out OF_EARLY_FLATTRE is anyway dependent on OF, and thus I
> > feel this check is redundant. I see all the Kconfig options which enables
> > both of these flags don't explicitly mention this dependency.
> >
> > >
> > > Separately, I wonder if the "select OF if !ACPI" is even needed. It doesn't
> > > hurt anything to leave it, but it seems like any config that doesn't
> > > independently select either ACPI or OF is broken for reasons unrelated
> > > to Hyper-V. I'm OK with leaving the select of OF if you want, so I'm
> > > more just wondering than asserting it should be removed. I didn't
> > > see "select OF if !ACPI" anywhere else in the Kconfig files, and it
> > > seems like Hyper-V would not be the only environment where this
> > > is the expectation.
> >
> > Ok I can remove the !ACPI dependency. Hope kernel size increase due to both
> > the code compiled in shouldn't be problem for ACPI systems.
> > And here if config doesn't select ACPI or OF it will assume OF, which is
> > better then selecting none of them.
> >
> >
> > To address both of your comments I feel below will be sufficient:
> > select OF
> > select OF_EARLY_FLATTRE
>
> Actually, that's not what I was thinking. :-) I was thinking for the Hyper-V
> Kconfig to be silent on selecting OF, just like it is silent on selecting ACPI.
> Whoever is configuring the kernel build would separately be selecting
> ACPI, or OF, or both, depending on their needs. I don't think the Hyper-V
> Kconfig should always be selecting OF, because of the reason you noted --
> it drags in code that is not needed for normal VTL 0 usage. If you take
> that approach, then
>
> select OF_EARLY_FLATTREE if OF
>
> is appropriate.

Thanks for clarifying, I will fix this in next vesrion.

Regards,
Saurabh

>
> Michael
>
>
>
> >
> >
> > Regards,
> > Saurabh
> >
> > >
> > > Michael