Re: [Qemu-devel] [PATCH 0/6] Add UUID command-line option

[Gleb Natapov <gleb@qumranet.com>]

[-- Attachment #1: Type: text/plain, Size: 3781 bytes --]

On Sun, Aug 24, 2008 at 04:35:00PM +0300, Blue Swirl wrote:
> >  > >  >               UUID is not architecture specific, so it should use the
> >  > >  > main structure instead if the architecture specific substructure
> >  > >  > (nvram_arch*). Adding UUID to unused fields will not break anything.
> >  > >
> >  > > Most info in ohwcfg_v3_t are not needed (or can be obtained by other
> >  > >  means) by PC BIOS, so there is no point in coping the whole structure
> >  > >  into BIOS. Of cause BIOS don't have to copy entire ohwcfg_v3_t, but
> >  >
> >  > For example, Bochs seems to use i440fx registers to determine the
> >  > available physical memory. This could be changed to use the
> >  > configuration structure instead. It's a matter of taste, but I would
> >  > find this an improvement.
> >
> > If by Bochs you mean Bochs BIOS then this is not the case. There are
> >  standard CMOS locations where amount of available memory is specified.
> >  Qemu initialize those locations and BIOS reads them. I don't think this is
> >  going to change since Bochs BIOS is used not only by Qemu. If only Bochs
> >  will move to firmware interface too...
> 
> If we can push UUID or ACPI patches, we can push just as easily
> configuration ROM and related interface patches. Of course Bochs devs
> could want other changes.
I don't what to speak on behalf of bochs developers, but I am not sure
about "as easily" part. Why change something that works for both qemu and
bochs to qemu specific way and guard it with ifdefs.

> 
> >  > >  access only required fields by reading only specific offsets, but then the
> >  > >  interface will be exactly like the one I proposed with only difference
> >  > >  that instead of specifying magic value (like 1 for reading UUID in my
> >  > >  patch series), BIOS will have to specify magic offset (like 0xE0).
> >  >
> >  > There is no need for a magic offset, ohwcfg_v3_t is designed to be
> >  > included even from asm.
> >  >
> >
> > The thing is I don't want to copy the whole ROM into BIOS during boot.
> >  As far as I see on other platforms ROM is memory mapped, so no copying
> >  is required and this was you first proposal, but if we are going to use
> >  port IO to access ROM then it will either have to be copied into main
> >  memory and then accessed, or only required fields  will have to be
> >  copied and that mean coping data from magic offsets.
> 
> I'd still prefer to use a memory mapped interface, but I was told that
> this would not be easy to access from 16 bit code. It's equally
> possible to make the ROM indexed even on Sparc32 or maybe even add a
> hidden m48t59 to PC.
> 
> Maybe we mean different things with a magic offset. Bochs BIOS sources
> can include firmware_abi.h unchanged, then use the #defined offsets to
> access either a memory-mapped ROM or IO port version. To my mind this
> does not mean using any magic offsets.

By magic offset I mean 0xE0 in the code below that will be added to BIOS to
read UUID in case we will use firmware interface:

     outl(QEMU_CFG_PORT, 0xE0 /* why not 1 ? */);
     for(i=0;i<16;i++)
       uuid[i] = intb(QEMU_CFG_PORT);

To read CPU frequency we should do:
     outl(QEMU_CFG_PORT, some other offset /* why not 2 */);

To read acpi table:
     outl(QEMU_CFG_PORT, one more offset /* why not 3 */)

To read one more parameter
     outl(QEMU_CFG_PORT, this time the offset depends on the size of previously read acpi table)


The patch that make bochs BIOS to use my proposed interface is attached
just to clarify the interface.

So let me ask you a question. What's actually the advantage of using firmware
ABI? It surely don't make our job easier as we have to extend it to get
information we need and we don't need any information it provides.

--
			Gleb.

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2828 bytes --]

commit ae12d85485bd7bf0434f13619f49274ad9c18d93
Author: Gleb Natapov <gleb@qumranet.com>
Date:   Sun Aug 24 10:59:25 2008 +0300

    Use new io port interface for qemu<->BIOS communication.

diff --git a/bios/rombios32.c b/bios/rombios32.c
index 2dc1d25..bdbd0fd 100755
--- a/bios/rombios32.c
+++ b/bios/rombios32.c
@@ -443,25 +443,49 @@ void wrmsr_smp(uint32_t index, uint64_t val)
     p->ecx = 0;
 }
 
+#ifdef BX_QEMU
+#define QEMU_CFG_PORT 0x1234
+enum {
+    QEMU_CFG_SIGNATURE,
+    QEMU_CFG_UUID,
+    QEMU_CFG_CPUSPEED,
+    QEMU_CFG_MAX_ENTRY
+};
+
+int qemu_cfg_port;
+
+void qemu_cfg_select(int f)
+{
+    outb(QEMU_CFG_PORT, f);
+}
+
+int qemu_cfg_port_probe()
+{
+    char *sig = "QEMU";
+    int i;
+
+    qemu_cfg_select(QEMU_CFG_SIGNATURE);
+
+    for (i = 0; i < 4; i++)
+        if (inb(QEMU_CFG_PORT) != sig[i])
+            return 0;
+
+    return 1;
+}
+
+void qemu_cfg_read(uint8_t *buf, int len)
+{
+    while (len--)
+        *(buf++) = inb(QEMU_CFG_PORT);
+}
+#endif
+
 void uuid_probe(void)
 {
 #ifdef BX_QEMU
-    uint32_t eax, ebx, ecx, edx;
-
-    // check if backdoor port exists
-    asm volatile ("outl %%eax, %%dx"
-        : "=a" (eax), "=b" (ebx), "=c" (ecx), "=d" (edx)
-        : "a" (0x564d5868), "c" (0xa), "d" (0x5658));
-    if (ebx == 0x564d5868) {
-        uint32_t *uuid_ptr = (uint32_t *)bios_uuid;
-        // get uuid
-        asm volatile ("outl %%eax, %%dx"
-            : "=a" (eax), "=b" (ebx), "=c" (ecx), "=d" (edx)
-            : "a" (0x564d5868), "c" (0x13), "d" (0x5658));
-        uuid_ptr[0] = eax;
-        uuid_ptr[1] = ebx;
-        uuid_ptr[2] = ecx;
-        uuid_ptr[3] = edx;
+    if(qemu_cfg_port) {
+        qemu_cfg_select(QEMU_CFG_UUID);
+        qemu_cfg_read(bios_uuid, 16);
     } else
 #endif
     {
@@ -1830,6 +1854,24 @@ smbios_type_3_init(void *start)
     return start+2;
 }
 
+static int qery_cpu_freq(void)
+{
+#ifdef BX_QEMU
+    int cpu_speed;
+
+    if(!qemu_cfg_port)
+        return 0;
+
+    qemu_cfg_select(QEMU_CFG_CPUSPEED);
+    qemu_cfg_read((uint8_t*)&cpu_speed, 4);
+
+    return cpu_speed;
+#else
+    return 0;
+#endif
+}
+
+
 /* Type 4 -- Processor Information */
 static void *
 smbios_type_4_init(void *start, unsigned int cpu_number)
@@ -1852,8 +1894,8 @@ smbios_type_4_init(void *start, unsigned int cpu_number)
     p->voltage = 0;
     p->external_clock = 0;
 
-    p->max_speed = 0; /* unknown */
-    p->current_speed = 0; /* unknown */
+    p->max_speed = 0x0FA0; /* 4000 MHZ */
+    p->current_speed = qery_cpu_freq(); /* unknown */
 
     p->status = 0x41; /* socket populated, CPU enabled */
     p->processor_upgrade = 0x01; /* other */
@@ -2058,6 +2100,10 @@ void rombios32_init(void)
 
     init_smp_msrs();
 
+#ifdef BX_QEMU
+    qemu_cfg_port = qemu_cfg_port_probe();
+#endif
+
     ram_probe();
 
     cpu_probe();