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

[Michael Kelley (LINUX)]

From: Saurabh Singh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
> > > ---
> > > [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.

Michael



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