linux-kernel - Re: platform/x86: p2sb: Allow p2sb_bar() calls during PCI device probe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zzyk5r22AS/Feg7z@goliath>
Date: Tue, 19 Nov 2024 14:47:02 +0000
From: "Daniel Walker (danielwa)" <danielwa@...co.com>
To: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
CC: Hans de Goede <hdegoede@...hat.com>, Shinichiro Kawasaki
	<shinichiro.kawasaki@....com>, Ilpo J�rvinen
	<ilpo.jarvinen@...ux.intel.com>, Klara Modin <klarasmodin@...il.com>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, Danil Rybakov
	<danilrybakov249@...il.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "xe-linux-external(mailer list)"
	<xe-linux-external@...co.com>
Subject: Re: platform/x86: p2sb: Allow p2sb_bar() calls during PCI device
 probe

On Tue, Nov 19, 2024 at 11:41:49AM +0200, Andy Shevchenko wrote:
> On Mon, Nov 18, 2024 at 05:15:17PM +0000, Daniel Walker (danielwa) wrote:
> > On Mon, Nov 18, 2024 at 05:00:52PM +0100, Hans de Goede wrote:
> > > On 18-Nov-24 4:55 PM, Andy Shevchenko wrote:
> > > > On Mon, Nov 18, 2024 at 02:35:44PM +0000, Daniel Walker (danielwa) wrote:
> > > >> On Mon, Nov 18, 2024 at 03:49:32PM +0200, Andy Shevchenko wrote:
> > > >>> On Mon, Nov 18, 2024 at 01:32:55PM +0000, Daniel Walker (danielwa) wrote:
> > > >>>> On Mon, Nov 18, 2024 at 03:24:20PM +0200, Andy Shevchenko wrote:
> > > >>>>> On Mon, Nov 18, 2024 at 12:40:16PM +0000, Daniel Walker (danielwa) wrote:
> 
> ...
> 
> > > >>>>> Are you referring to LPC GPIO?
> > > >>>>  
> > > >>>>  I don't know the hardware well enough to say for certain. It's whatever device 8086:19dd is.
> > > >>>
> > > >>> This is device which represents p2sb. It's not a GPIO device you are talking
> > > >>> about for sure. You can send privately more detailed info in case this is shouldn't
> > > >>> be on public to me to understand what would be the best approach to fix your issue.
> > > >>
> > > >> Here's a comment,
> > > >>
> > > >> /* INTEL Denverton GPIO registers are accessible using SBREG_BAR(bar 0) as base */
> > > >>
> > > >> We have gpio wired to an FPGA and I believe the gpio line is used to reset the
> > > >> fpga.
> > > >>
> > > >> So the pci resources attached to 8086:19dd can be used to access gpio of some
> > > >> type. 
> > > >>
> > > >> I'm not a pci expert but on the 19bb device bar 0 we use the below offset to manipulate
> > > >> the gpio,
> > > >>
> > > >> #define INTEL_GPIO_REG_RESET_OFFSET          0xC50578
> > > >>
> > > >> The comments suggest this is gpio 6. I would seems your reaction would be that
> > > >> there is no gpio on the 19dd device. Maybe our driver is access gpio thru p2sb
> > > >> or something like that.
> > > >>
> > > >> Does the offset above make sense to you in the context of the p2sb ?
> > > > 
> > > > Yes, everything makes sense. Please, enable lpc_ich driver and forget about
> > > > talking to the p2sb memory mapped IO.
> > > > 
> > > > /* Offset data for Denverton GPIO controllers */
> > > > static const resource_size_t dnv_gpio_offsets[DNV_GPIO_NR_RESOURCES] = {
> > > > 	[DNV_GPIO_NORTH] = 0xc20000,
> > > > 	[DNV_GPIO_SOUTH] = 0xc50000,
> > > > };
> > > > 
> > > > So, you are using a pin from the Community "South" of the on-die Denverton GPIO.
> > > > 
> > > > In EDS this called GPIO_6, while in the driver it's pin 88, i.e. SMB3_IE0_DATA.
> > > > 
> > > > You will need to
> > > > - enable lpc_ich driver (CONFIG_LPC_ICH)
> > > > - enable Intel Denverton pin control driver (CONFIG_PINCTRL_DENVERTON)
> > > > - replace your custom approach to:
> > > >   - GPIO lookup table
> > > >   - proper GPIO APIs, as gpiod_get() or alike
> > > > 
> > > > See how it was done in the current kernel code:
> > > > 
> > > > 8241b55f1ded ("drm/i915/dsi: Replace poking of VLV GPIOs behind the driver's back")
> > > > a6c80bec3c93 ("leds: simatic-ipc-leds-gpio: Add GPIO version of Siemens driver")
> > > > 
> > > > Hans, there will be no need to fix anything if they implement correct access
> > > > to the GPIO, i.e. via driver and board code with GPIO lookup tables.
> > > 
> > > Agreed, still I'm not sure how I feel about us hiding the previously unhidden P2SB.
> > > 
> > > OTOH I guess it may have only been unhidden in the BIOS to make the hack they
> > > are using possible in the first place.
> > 
> > From a flexibility POV I would suggest if you can not hide it if it's not already
> > hidden by the BIOS that would be better since some company may have a good
> > reason to make a custom driver or to export the pci device to userspace thru
> > UIO.
> 
> The previous emails used wrong terminology, the hidden device is left hidden, and
> all the opposite: unhidden is not touched in this sense.
> 
> The problem there that for the initially unhidden devices we call pci_stop_...
> which seems incorrect and needs to be fixed, indeed.
> 
> > The current situation is you can't make a custom driver if p2sb is enable
> > with this additional patch even if you unhide the device inside the BIOS.
> 
> Yeah, but I do not consider that is anyhow related to upstream.
> 

Not true. Have you used the UIO system ? You can make a custom userspace driver
for a pci device with zero kernel changes. We have a custom LKM , but we could
easily have done it with UIO. With the PCI device completely gone, there is no
way to use UIO to make a userspace driver.

Daniel