On Sun, Apr 14, 2024 at 11:51:33AM -0400, Kevin O'Connor wrote:
> 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.
Ah, I missed that vgahook_setup() checks for NULL. Out of curiosity,
is it possible to limit this support to a particular hardware vendor
instead of by existence of file?
-Kevin
>
> 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
