Hi,

we had for VBox@Genode/Nova to implement our own version of the IO
Monitor. During usage we detected some minor issues in the ACPI, PCI and
VGA device model between announced/registered IO ports access width and
its actual access width usage.

acpi.patch
- The access width of PM1a_EVT_OFFSET, PM1a_CTL_OFFSET, PM_TMR_OFFSET
are announced as 1 Byte, however are according to the ACPI Spec 4.0a
Chapter 4.7.3++ are the half of the PM1_EVT_LEN resp. PM1_CNT_LEN. We
adjusted the model accordingly.

acpi_more.patch
- The BAT_INDEX, BAT_DATA, SYSI_INDEX and SYSI_DATA is accessed 4 byte
wise and not only 1 byte.
- THE DEBUG_HEX, DEBUG_CHR is also accessed 4 byte wise. However to fix
that the DEBUG_CHR port must be changed. I don't know whether this has
influence to your debugging tools if they expect DEBUG_CHR to be 3001
instead of 3004?

devpci.patch
- port 0x0cf8 is access 4 byte wise

vga.patch
- several ports are accessed 2 byte wise instead of 1 byte. The changes
to the ports 0x1ce and 0x1cf are potentially not appropriate. They are
only 1 byte apart, but each port is accessed 2 byte wise, so I don't
know if this part of the patch is valid. Please check.


And some minor fixes regarding compile errors:

vmx_c++.patch
- The include header does not compile with C++ features enabled.

vmmdev.patch
- If VBOX_WITH_HGCM is not set, still some HGCM code is tried to be
compiled for VMMDev.cpp which fails. Set #ifdef accordingly.



Please review the patches and tell me whether they/some of them make
sense to you. They are in the first place non-critical, however our IOM
implementation complains/deny access by guests to device models if the
i/O access width is bigger than registered by the device model.

The patches are for virtualbox 4.2.16. If wanted I could also re-prepare
it so that they apply for the current version of VirtualBox.

Regards,

Alexander Boettcher.

The patches are provided according to the signed Oracle contributors
agreement by Genode Labs GmbH.
+++ src/VBox/Devices/PC/DevACPI.cpp
@@ -438,7 +438,9 @@
     uint32_t            u32GPE0BLK;             /**< port addr of gen-purp event 0 regs block */
     uint32_t            u32GPE1BLK;             /**< port addr of gen-purp event 1 regs block */
     uint8_t             u8PM1EVTLEN;            /**< bytes decoded by PM1a_EVT_BLK. >= 4 */
+#define ACPI_PM1_EVT_LEN 0x4
     uint8_t             u8PM1CTLLEN;            /**< bytes decoded by PM1b_CNT_BLK. >= 2 */
+#define ACPI_PM1_CNT_LEN 0x2
     uint8_t             u8PM2CTLLEN;            /**< bytes decoded by PM2_CNT_BLK. >= 1 or 0 */
     uint8_t             u8PMTMLEN;              /**< bytes decoded by PM_TMR_BLK. ==4 */
     uint8_t             u8GPE0BLKLEN;           /**< bytes decoded by GPE0_BLK. %2==0 */
@@ -1842,10 +1844,10 @@
     } while (0)
 #define L       (GPE0_BLK_LEN / 2)
 
-    R(PM1a_EVT_OFFSET+2, 1, acpiPM1aEnWrite,       acpiPm1aEnRead,      "ACPI PM1a Enable");
-    R(PM1a_EVT_OFFSET,   1, acpiPM1aStsWrite,      acpiPm1aStsRead,     "ACPI PM1a Status");
-    R(PM1a_CTL_OFFSET,   1, acpiPM1aCtlWrite,      acpiPm1aCtlRead,     "ACPI PM1a Control");
-    R(PM_TMR_OFFSET,     1, NULL,                  acpiPMTmrRead,       "ACPI PM Timer");
+    R(PM1a_EVT_OFFSET+2, ACPI_PM1_EVT_LEN / 2, acpiPM1aEnWrite,       acpiPm1aEnRead,      "ACPI PM1a Enable");
+    R(PM1a_EVT_OFFSET, ACPI_PM1_EVT_LEN / 2, acpiPM1aStsWrite,      acpiPm1aStsRead,     "ACPI PM1a Status");
+    R(PM1a_CTL_OFFSET, ACPI_PM1_CNT_LEN, acpiPM1aCtlWrite,      acpiPm1aCtlRead,     "ACPI PM1a Control");
+    R(PM_TMR_OFFSET,     4, NULL,                  acpiPMTmrRead,       "ACPI PM Timer");
     R(GPE0_OFFSET + L,   L, acpiGpe0EnWrite,       acpiGpe0EnRead,      "ACPI GPE0 Enable");
     R(GPE0_OFFSET,       L, acpiGpe0StsWrite,      acpiGpe0StsRead,     "ACPI GPE0 Status");
 #undef L
@@ -2189,8 +2191,8 @@
     fadt.u32PMTMRBLK          = RT_H2LE_U32(acpiCalcPmPort(pThis, PM_TMR_OFFSET));
     fadt.u32GPE0BLK           = RT_H2LE_U32(acpiCalcPmPort(pThis, GPE0_OFFSET));
     fadt.u32GPE1BLK           = RT_H2LE_U32(acpiCalcPmPort(pThis, GPE1_OFFSET));
-    fadt.u8PM1EVTLEN          = 4;
-    fadt.u8PM1CTLLEN          = 2;
+    fadt.u8PM1EVTLEN          = ACPI_PM1_EVT_LEN;
+    fadt.u8PM1CTLLEN          = ACPI_PM1_CNT_LEN;
     fadt.u8PM2CTLLEN          = 0;
     fadt.u8PMTMLEN            = 4;
     fadt.u8GPE0BLKLEN         = GPE0_BLK_LEN;
+++ src/VBox/Devices/PC/DevACPI.cpp
@@ -68,7 +68,7 @@
 
 
 #define DEBUG_HEX       0x3000
-#define DEBUG_CHR       0x3001
+#define DEBUG_CHR       0x3004
 
 #define PM_TMR_FREQ     3579545
 /* Default base for PM PIIX4 device */
@@ -3254,13 +3256,13 @@
     } while (0)
     R(SMI_CMD,        1, acpiSmiWrite,          NULL,                "ACPI SMI");
 #ifdef DEBUG_ACPI
-    R(DEBUG_HEX,      1, acpiDhexWrite,         NULL,                "ACPI Debug hex");
-    R(DEBUG_CHR,      1, acpiDchrWrite,         NULL,                "ACPI Debug char");
+    R(DEBUG_HEX,      4, acpiDhexWrite,         NULL,                "ACPI Debug hex");
+    R(DEBUG_CHR,      4, acpiDchrWrite,         NULL,                "ACPI Debug char");
 #endif
-    R(BAT_INDEX,      1, acpiBatIndexWrite,     NULL,                "ACPI Battery status index");
-    R(BAT_DATA,       1, NULL,                  acpiBatDataRead,     "ACPI Battery status data");
-    R(SYSI_INDEX,     1, acpiSysInfoIndexWrite, NULL,                "ACPI system info index");
-    R(SYSI_DATA,      1, acpiSysInfoDataWrite,  acpiSysInfoDataRead, "ACPI system info data");
+    R(BAT_INDEX,      4, acpiBatIndexWrite,     NULL,                "ACPI Battery status index");
+    R(BAT_DATA,       4, NULL,                  acpiBatDataRead,     "ACPI Battery status data");
+    R(SYSI_INDEX,     4, acpiSysInfoIndexWrite, NULL,                "ACPI system info index");
+    R(SYSI_DATA,      4, acpiSysInfoDataWrite,  acpiSysInfoDataRead, "ACPI system info data");
     R(ACPI_RESET_BLK, 1, acpiResetWrite,        NULL,                "ACPI Reset");
 #undef R
 
+++ src/VBox/Devices/Bus/DevPCI.cpp
@@ -2279,7 +2280,7 @@
     /*
      * Register I/O ports and save state.
      */
-    rc = PDMDevHlpIOPortRegister(pDevIns, 0x0cf8, 1, NULL, pciIOPortAddressWrite, pciIOPortAddressRead, NULL, NULL, "i440FX (PCI)");
+    rc = PDMDevHlpIOPortRegister(pDevIns, 0x0cf8, 4, NULL, pciIOPortAddressWrite, pciIOPortAddressRead, NULL, NULL, "i440FX (PCI)");
     if (RT_FAILURE(rc))
         return rc;
     rc = PDMDevHlpIOPortRegister(pDevIns, 0x0cfc, 4, NULL, pciIOPortDataWrite, pciIOPortDataRead, NULL, NULL, "i440FX (PCI)");
+++ src/VBox/Devices/Graphics/DevVGA.cpp
@@ -6024,10 +5984,10 @@
 #endif /* VBOX_WITH_HGSMI */
 
 #ifdef CONFIG_BOCHS_VBE
-    rc = PDMDevHlpIOPortRegister(pDevIns,  0x1ce,  1, NULL, vgaIOPortWriteVBEIndex, vgaIOPortReadVBEIndex, NULL, NULL, "VGA/VBE - Index");
+    rc = PDMDevHlpIOPortRegister(pDevIns,  0x1ce,  2, NULL, vgaIOPortWriteVBEIndex, vgaIOPortReadVBEIndex, NULL, NULL, "VGA/VBE - Index");
     if (RT_FAILURE(rc))
         return rc;
-    rc = PDMDevHlpIOPortRegister(pDevIns,  0x1cf,  1, NULL, vgaIOPortWriteVBEData, vgaIOPortReadVBEData, NULL, NULL, "VGA/VBE - Data");
+    rc = PDMDevHlpIOPortRegister(pDevIns,  0x1cf,  2, NULL, vgaIOPortWriteVBEData, vgaIOPortReadVBEData, NULL, NULL, "VGA/VBE - Data");
     if (RT_FAILURE(rc))
         return rc;
 #endif /* CONFIG_BOCHS_VBE */
@@ -6470,7 +6430,7 @@
     /*
      * Register I/O Port for the VBE BIOS Extra Data.
      */
-    rc = PDMDevHlpIOPortRegister(pDevIns, VBE_EXTRA_PORT, 1, NULL, vbeIOPortWriteVBEExtra, vbeIOPortReadVBEExtra, NULL, NULL, "VBE BIOS Extra Data");
+    rc = PDMDevHlpIOPortRegister(pDevIns, VBE_EXTRA_PORT, 2, NULL, vbeIOPortWriteVBEExtra, vbeIOPortReadVBEExtra, NULL, NULL, "VBE BIOS Extra Data");
     if (RT_FAILURE(rc))
         return rc;
 #endif /* VBE_NEW_DYN_LIST */
@@ -6478,7 +6438,7 @@
     /*
      * Register I/O Port for the BIOS Logo.
      */
-    rc = PDMDevHlpIOPortRegister(pDevIns, LOGO_IO_PORT, 1, NULL, vbeIOPortWriteCMDLogo, vbeIOPortReadCMDLogo, NULL, NULL, "BIOS Logo");
+    rc = PDMDevHlpIOPortRegister(pDevIns, LOGO_IO_PORT, 2, NULL, vbeIOPortWriteCMDLogo, vbeIOPortReadCMDLogo, NULL, NULL, "BIOS Logo");
     if (RT_FAILURE(rc))
         return rc;
 
+++ include/VBox/vmm/hwacc_vmx.h
@@ -1338,10 +1338,10 @@
        ".byte    0xF3, 0x0F, 0xC7, 0x34, 0x24  # VMXON [esp]    \n\t"
        "ja       2f                                             \n\t"
        "je       1f                                             \n\t"
-       "movl     $"STR(VERR_VMX_INVALID_VMXON_PTR)", %0         \n\t"
+       "movl     $" STR(VERR_VMX_INVALID_VMXON_PTR)", %0        \n\t"
        "jmp      2f                                             \n\t"
        "1:                                                      \n\t"
-       "movl     $"STR(VERR_VMX_GENERIC)", %0                   \n\t"
+       "movl     $" STR(VERR_VMX_GENERIC)", %0                  \n\t"
        "2:                                                      \n\t"
        "add      $8, %%esp                                      \n\t"
        :"=rm"(rc)
@@ -1418,7 +1418,7 @@
        "push    %2                                              \n\t"
        ".byte   0x66, 0x0F, 0xC7, 0x34, 0x24  # VMCLEAR [esp]   \n\t"
        "jnc     1f                                              \n\t"
-       "movl    $"STR(VERR_VMX_INVALID_VMCS_PTR)", %0           \n\t"
+       "movl    $" STR(VERR_VMX_INVALID_VMCS_PTR)", %0          \n\t"
        "1:                                                      \n\t"
        "add     $8, %%esp                                       \n\t"
        :"=rm"(rc)
@@ -1466,7 +1466,7 @@
        "push    %2                                              \n\t"
        ".byte   0x0F, 0xC7, 0x34, 0x24  # VMPTRLD [esp]         \n\t"
        "jnc     1f                                              \n\t"
-       "movl    $"STR(VERR_VMX_INVALID_VMCS_PTR)", %0           \n\t"
+       "movl    $" STR(VERR_VMX_INVALID_VMCS_PTR)", %0          \n\t"
        "1:                                                      \n\t"
        "add     $8, %%esp                                       \n\t"
        :"=rm"(rc)
@@ -1520,10 +1520,10 @@
        ".byte  0x0F, 0x79, 0xC2        # VMWRITE eax, edx       \n\t"
        "ja     2f                                               \n\t"
        "je     1f                                               \n\t"
-       "movl   $"STR(VERR_VMX_INVALID_VMCS_PTR)", %0            \n\t"
+       "movl   $" STR(VERR_VMX_INVALID_VMCS_PTR)", %0           \n\t"
        "jmp    2f                                               \n\t"
        "1:                                                      \n\t"
-       "movl   $"STR(VERR_VMX_INVALID_VMCS_FIELD)", %0          \n\t"
+       "movl   $" STR(VERR_VMX_INVALID_VMCS_FIELD)", %0         \n\t"
        "2:                                                      \n\t"
        :"=rm"(rc)
        :"0"(VINF_SUCCESS),
@@ -1607,14 +1607,14 @@
     int rc = VINF_SUCCESS;
 # if RT_INLINE_ASM_GNU_STYLE
     __asm__ __volatile__ (
-       "movl   $"STR(VINF_SUCCESS)", %0                          \n\t"
+       "movl   $" STR(VINF_SUCCESS)", %0                         \n\t"
        ".byte  0x0F, 0x78, 0xc2        # VMREAD eax, edx         \n\t"
        "ja     2f                                                \n\t"
        "je     1f                                                \n\t"
-       "movl   $"STR(VERR_VMX_INVALID_VMCS_PTR)", %0             \n\t"
+       "movl   $" STR(VERR_VMX_INVALID_VMCS_PTR)", %0            \n\t"
        "jmp    2f                                                \n\t"
        "1:                                                       \n\t"
-       "movl   $"STR(VERR_VMX_INVALID_VMCS_FIELD)", %0           \n\t"
+       "movl   $" STR(VERR_VMX_INVALID_VMCS_FIELD)", %0          \n\t"
        "2:                                                       \n\t"
        :"=&r"(rc),
         "=d"(*pData)
+++ src/VBox/Devices/VMMDev/VMMDev.cpp	2013-10-17 13:02:16.787071776 +0200
@@ -1854,7 +1854,6 @@
             }
             break;
         }
-#endif /* VBOX_WITH_HGCM */
 
         case VMMDevReq_HGCMCancel:
         {
@@ -1899,6 +1898,7 @@
             }
             break;
         }
+#endif /* VBOX_WITH_HGCM */
 
         case VMMDevReq_VideoAccelEnable:
         {
@@ -3341,8 +3341,10 @@
     /* disabled statistics updating */
     pThis->u32LastStatIntervalSize = 0;
 
+#ifdef VBOX_WITH_HGCM
     /* Clear the "HGCM event enabled" flag so the event can be automatically reenabled.  */
     pThis->u32HGCMEnabled = 0;
+#endif
 
     /*
      * Clear the event variables.

Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev

Reply via email to