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: <Zzt2JNchK9A0pSlZ@goliath>
Date: Mon, 18 Nov 2024 17:15:17 +0000
From: "Daniel Walker (danielwa)" <danielwa@...co.com>
To: Hans de Goede <hdegoede@...hat.com>
CC: Andy Shevchenko <andriy.shevchenko@...ux.intel.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 Mon, Nov 18, 2024 at 05:00:52PM +0100, Hans de Goede wrote:
> Hi,
> 
> 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 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.

In our case it seems like we could use the already existing solution with
pinctrl, but others may not be able to do that or may not want to for different
reasons.

Daniel