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

[Saurabh Singh Sengar]

On Mon, Mar 13, 2023 at 12:26:56PM +0000, Michael Kelley (LINUX) wrote:
> 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.

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

Regards,
Saurabh

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