<div dir="ltr"><div dir="ltr"><div>Hi Michal,</div><div><br></div><div>Thanks for taking a look.</div><div><br></div><div>I agree that the behavior should not change and I don't believe it does. I removed the call to acpiR3PrepareHeader (as well as the surrounding lines) because it was dead code. Looking at the prior acpiR3SetupCust function:</div><div><br></div><div>/** Custom Description Table */</div><div>static void acpiR3SetupCust(ACPIState *pThis, RTGCPHYS32 addr)</div><div>{</div><div>    ACPITBLCUST cust;</div><div><br></div><div>    /* First the ACPI version 1 version of the structure. */</div><div>    memset(&cust, 0, sizeof(cust));</div><div>    acpiR3PrepareHeader(pThis, &cust.header, "CUST", sizeof(cust), 1);</div><div><br></div><div>    memcpy(cust.header.au8OemTabId, pThis->au8OemTabId, 8);</div><div>    cust.header.u32OemRevision = RT_H2LE_U32(pThis->u32OemRevision);</div><div>    cust.header.u8Checksum = acpiR3Checksum((uint8_t *)&cust, sizeof(cust));</div><div><br></div><div>    acpiR3PhysCopy(pThis, addr, pThis->pu8CustBin, pThis->cbCustBin);</div><div>}</div><div><br></div><div>One can see that the cust structure is allocated locally on the first line, setup, but then discarded when the function exits without ever being used. The only code in the function that does anything is the final line.</div><div><br></div><div>The custom table header that is actually used is the one provided by the user, which is loaded and present in the table itself (pThis->pu8CustBin). This behavior has been retained in the updated function:</div><div><br></div><div>/** Custom Description Table */</div><div>static void acpiR3SetupCust(ACPIState *pThis, RTGCPHYS32 addr, uint8_t custTableIdx)</div><div>{</div><div>    Assert(custTableIdx < MAX_CUST_TABLES);</div><div>    acpiR3PhysCopy(pThis, addr, pThis->pu8CustBin[custTableIdx], pThis->cbCustBin[custTableIdx]);</div><div>}</div><div><br></div><div>With respect to the incoming data, which part did you notice was copied differently? The intention is definitely to retain the existing behavior so if there's a difference there, it's unintended.</div><div><br></div><div>In regard to use cases, one example is the requirement to now pass through both SLIC and MSDM tables when using (legally) using a Windows 10 OEM edition as a guest on the machine that it was licensed on (this was the motivation for the original custom table mechanism for SLIC passthrough for Windows 7/8 - see <a href="https://www.virtualbox.org/ticket/9231">https://www.virtualbox.org/ticket/9231</a>). Reducing the detection surface for dynamic analysis of malware that actively detects sandboxes is another.</div><div><br></div><div>Regards,</div></div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Sep 4, 2018 at 10:45 PM Michal Necasek <<a href="mailto:michal.necasek@oracle.com">michal.necasek@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
    Hi,<br>
<br>
  Thanks for the patch! I quickly looked over it and it looks to me that <br>
the behavior of existing VMs with the <br>
VBoxInternal/Devices/acpi/0/Config/CustomTable keys set will subtly <br>
change because the call to<br>
<br>
acpiR3PrepareHeader(pThis, &cust.header, "CUST", sizeof(cust), 1);<br>
<br>
is no longer done and the incoming data is copied a bit differently. Can <br>
you explain why that's not done or why it can't cause any problems?<br>
<br>
  Wouldn't it be better to keep the existing behavior for CustomTable <br>
and just add CustomTable1/2/3 with slightly different behavior?<br>
<br>
  Also, do you have a concrete example of ACPI tables that one might <br>
want to pass to a VM this way?<br>
<br>
<br>
     Regards,<br>
       Michal<br>
<br>
On 8/24/2018 9:14 PM, Canardos . wrote:<br>
> Hi Devs,<br>
> <br>
> This patch expands on work done several years ago, extending support for <br>
> custom ACPI tables from a single table to four tables, and potentially N <br>
> tables in future.<br>
> <br>
> The changes assist malware researchers to better emulate a physical <br>
> machine (although increasing the aggregate allowable table size will <br>
> assist further) as well as those with valid OEM licenses having issues <br>
> property passing through both SLIC and license tables.<br>
> <br>
> I limited the implementation to four custom tables given current VBox <br>
> limitations on the aggregate size of ACPI tables, but the code changes <br>
> support N tables.<br>
> <br>
> The new configuration keys are "CustomTable0..3". Legacy behavior has <br>
> been maintained, with any existing config entries for "CustomTable", or <br>
> "SLICTable" being used in the absence of a "CustomTable0" entry.<br>
> <br>
> The changes have been tested on Debian 9.5 (kernel 4.9.0-7/amd64) and <br>
> Manjaro (kernel 4.14.65-1/amd64) hosts with 32 and 64-bit Linux guests.<br>
> <br>
> I've endeavored to reflect the code style of the file I was working in, <br>
> please let me know if anything is amiss.<br>
> <br>
> All attached code is released under the MIT License.<br>
> <br>
> <br>
> <br>
> _______________________________________________<br>
> vbox-dev mailing list<br>
> <a href="mailto:vbox-dev@virtualbox.org" target="_blank">vbox-dev@virtualbox.org</a><br>
> <a href="https://www.virtualbox.org/mailman/listinfo/vbox-dev" rel="noreferrer" target="_blank">https://www.virtualbox.org/mailman/listinfo/vbox-dev</a><br>
> <br>
<br>
_______________________________________________<br>
vbox-dev mailing list<br>
<a href="mailto:vbox-dev@virtualbox.org" target="_blank">vbox-dev@virtualbox.org</a><br>
<a href="https://www.virtualbox.org/mailman/listinfo/vbox-dev" rel="noreferrer" target="_blank">https://www.virtualbox.org/mailman/listinfo/vbox-dev</a><br>
</blockquote></div>