All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Tomita Moeko <tomitamoeko@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] x86/pci: Check signature before assigning shadow ROM
Date: Fri, 17 Oct 2025 14:41:59 +0200	[thread overview]
Message-ID: <aPI5l0whaAIJGaSw@wunner.de> (raw)
In-Reply-To: <20251016081900.7129-1-tomitamoeko@gmail.com>

On Thu, Oct 16, 2025 at 04:19:00PM +0800, Tomita Moeko wrote:
> Recent IGD platforms without VBIOS or UEFI CSM support do not contain
> VGA ROM at 0xC0000. Check whether the VGA ROM region is a valid PCI
> option ROM with 0xAA55 signature before assigning the shadow ROM to
> the default PCI VGA controller.
[...]
> +++ b/arch/x86/pci/fixup.c
> @@ -357,6 +357,18 @@ static void pci_fixup_video(struct pci_dev *pdev)
>  	struct pci_bus *bus;
>  	u16 config;
>  	struct resource *res;
> +	void *rom;
> +	u16 sig;
> +
> +	/* Does VBIOS region contain a valid PCI ROM? */
> +	rom = memremap(0xC0000, sizeof(sig), MEMREMAP_WB);
> +	if (!rom)
> +		return;
> +
> +	memcpy(&sig, rom, sizeof(sig));
> +	memunmap(rom);
> +	if (sig != 0xAA55)
> +		return;
>  
>  	/* Is VGA routed to us? */
>  	bus = pdev->bus;

I have to ask again, in arch/x86/kernel/probe_roms.c:probe_roms(),
the signature is already verified.  If it doesn't match, the
video_rom_resource isn't added to iomem_resource.

Which makes me wonder, wouldn't it be sufficient to just do
something like:

	if (!lookup_resource(&iomem_resource, 0xC0000))
		return;

Another thought I have, I'd move the code you're inserting further
down, perhaps after the while-loop.  Actually the existing code
isn't very pretty, there should be a return after failure of the
vga_default_device checks and after the Command register check
so that the actual resource adjustment doesn't need to be indented.

Thanks,

Lukas

      reply	other threads:[~2025-10-17 12:42 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-16  8:19 [PATCH v2] x86/pci: Check signature before assigning shadow ROM Tomita Moeko
2025-10-17 12:41 ` Lukas Wunner [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aPI5l0whaAIJGaSw@wunner.de \
    --to=lukas@wunner.de \
    --cc=bhelgaas@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tomitamoeko@gmail.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.