On Sun, Apr 14, 2024 at 10:30:55AM +0000, Riku Viitanen via SeaBIOS wrote:
> VGAROMs on MXM graphics cards need certain int15h functions present
> to function properly.
>
> On HP EliteBook 8560w with coreboot and Quadro 2000M, this warning
> is displayed for 30 seconds, making every boot extremely slow:
>
> ERROR: Valid MXM Structure not found
> POST halted for 30 seconds, P-state limited to P10...
>
> This patch implements the minimum functionality to get rid of it: the
> functions 0 and 1, version 3.0 of the specification [1]. Older version
> 2.1 is not implemented.
>
> Functions are enabled only if romfile "mxm-30-sis" exists. These functions
> are specific to the MXM revision, but not the board or GPU. Some boards
> might require more of the optional functions to be implemented, however.
>
> - Function 0 returns information about supported functions and revision.
>
> - Function 1 returns a pointer to a MXM configuration structure in low
> memory. This is copied from romfile "mxm-30-sis". It can be extracted
> from vendor BIOS, by using this same interrupt. I wrote a tool [2] to do
> that from Linux.
>
> TEST: HP EliteBook 8560w boots without error and delay, graphics work.
>
> [1]: MXM Graphics Module Software Specification Version 3.0, Revision 1.1
> [2]: https://codeberg.org/Riku_V/mxmdump/
Thanks. In general it looks good to me. I have a few minor comments.
>
> Signed-off-by: Riku Viitanen riku.viita...@protonmail.com
> ---
> src/vgahooks.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/src/vgahooks.c b/src/vgahooks.c
> index 1f149532..6adf5b9b 100644
> --- a/src/vgahooks.c
> +++ b/src/vgahooks.c
> @@ -11,13 +11,16 @@
> #include "hw/pcidevice.h" // pci_find_device
> #include "hw/pci_ids.h" // PCI_VENDOR_ID_VIA
> #include "hw/pci_regs.h" // PCI_VENDOR_ID
> +#include "malloc.h" // malloc_low
> #include "output.h" // dprintf
> +#include "romfile.h" // struct romfile_s, romfile_find
> #include "string.h" // strcmp
> #include "util.h" // handle_155f, handle_157f
>
> #define VH_VIA 1
> #define VH_INTEL 2
> #define VH_SMI 3
> +#define VH_MXM30 4
>
> int VGAHookHandlerType VARFSEG;
>
> @@ -296,6 +299,77 @@ winent_mb6047_setup(struct pci_device *pci)
> SmiBootDisplay = 0x02;
> }
>
> +/****************************************************************
> + * MXM 3.0 hooks
> + ****************************************************************/
> +
> +void *MXM30SIS VARFSEG;
> +
> +/* Function 0: Return Specification Support Level */
> +static void
> +mxm30_F00(struct bregs *regs)
> +{
> + regs->ax = 0x005f; /* Success */
> + regs->bl = 0x30; /* MXM version 3.0 */
> + regs->cx = (1<<0) | (1<<1); /* Supported functions */
> + set_success(regs);
> +}
> +
> +/* Function 1: Return Pointer to MXM System Information Structure */
> +static void
> +mxm30_F01(struct bregs *regs)
> +{
> + switch (regs->cx) {
> + case 0x0030:
> + regs->ax = 0x005f; /* Success */
> + regs->es = (u32)GET_GLOBAL(MXM30SIS)/16;
> + regs->di = (u32)GET_GLOBAL(MXM30SIS)%16;
This should use SEG_LOW and LOWFLAT2LOW(MXM30SIS).
> + set_success(regs);
> + break;
> + default:
> + handle_155fXX(regs);
> + break;
> + }
> +}
> +
> +static void
> +mxm30_155f80(struct bregs *regs)
> +{
> + switch (regs->bx) {
> + case 0xff00: mxm30_F00(regs); break;
> + case 0xff01: mxm30_F01(regs); break;
> + default: handle_155fXX(regs); break;
> + }
> +}
> +
> +static void
> +mxm30_155f(struct bregs *regs)
> +{
> + switch (regs->al) {
> + case 0x80: mxm30_155f80(regs); break;
> + default: handle_155fXX(regs); break;
> + }
> +}
> +
> +static void
> +mxm30_setup(struct romfile_s *sis_file)
> +{
> + /* Load MXM System Information Structure into low memory */
> + MXM30SIS = malloc_low(sis_file->size);
This code does not check if sis_file is NULL, which could lead to
memory corruption.
Probably best to use romfile_loadfile(), check for non-null, and then
malloc_low(), manual memcpy(), and free() of the temporary space. As
it's preferable to avoid calls to free() on malloc_low() allocations -
as that can lead to memory fragmentation of the scarce "low" region.
> + if (!MXM30SIS) {
> + warn_noalloc();
> + return;
> + }
> + int ret = sis_file->copy(sis_file, MXM30SIS, sis_file->size);
> + if (ret < 0) {
> + free(MXM30SIS);
> + MXM30SIS = NULL;
> + return;
> + }
> +
> + VGAHookHandlerType = VH_MXM30;
> +}
> +
> /****************************************************************
> * Entry and setup
> ****************************************************************/
> @@ -313,6 +387,7 @@ handle_155f(struct bregs *regs)
> switch (htype) {
> case VH_VIA: via_155f(regs); break;
> case VH_INTEL: intel_155f(regs); break;
> + case VH_MXM30: mxm30_155f(regs); break;
> default: handle_155fXX(regs); break;
> }
> }
> @@ -340,6 +415,8 @@ vgahook_setup(struct pci_device *pci)
> if (!CONFIG_VGAHOOKS)
> return;
>
> + struct romfile_s *mxm30sis = romfile_find("mxm-30-sis");
> +
> if (strcmp(CBvendor, "KONTRON") == 0 && strcmp(CBpart, "986LCD-M") == 0)
> kontron_setup(pci);
> else if (strcmp(CBvendor, "GETAC") == 0 && strcmp(CBpart, "P470") == 0)
> @@ -352,4 +429,6 @@ vgahook_setup(struct pci_device *pci)
> via_setup(pci);
> else if (pci->vendor == PCI_VENDOR_ID_INTEL)
> intel_setup(pci);
> + else if (mxm30sis != NULL)
> + mxm30_setup(mxm30sis);
I think it would be preferable if mxm_setup() was passed the pci
device for uniformity, and have mxm_setup() call romfile_xxx() itself.
-Kevin
> }
> --
> 2.44.0
>
> _______________________________________________
> SeaBIOS mailing list -- seabios@seabios.org
> To unsubscribe send an email to seabios-le...@seabios.org
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
