On 02/15/17 21:09, Michael S. Tsirkin wrote: > On Wed, Feb 15, 2017 at 08:47:48PM +0100, Laszlo Ersek wrote:
[snip] >> For patches #1, #3, #4 and #5: >> >> Tested-by: Laszlo Ersek <ler...@redhat.com> >> >> I'll soon post the OVMF patches. >> >> Thanks! >> Laszlo > > > How do you feel about Igor's request to change WRITE_POINTER to add > offset in there, so guest can pass in the address of GUID and > not start of table? Would that be a lot of work to add? I think it's doable in practice: simply add a constant from the command itself, for passing the value back to QEMU, and also for saving the fw_cfg write commend for S3 resume time. But, I disagree with it from a design POV. Igor's point is: > Math complicates QEMU code though and not only QMEMU but AML code as > well. As I understand it, the goal is to push the addition to the firmware (which is "one place"), rather than having to implement it twice in QEMU, i.e., in two places ((a) native QEMU logic, (b) AML generator). Here's my counter-argument: (a) As I mentioned earlier, assume a complex communication structure between the guest OS and QEMU. Currently our shared structure consists of a single field (the GUID), but next time it might contain several fields. For such a multi-field shared structure, QEMU will have to do manual offsetting into the guest RAM anyway, for accessing fields F1, F2, and F3. We will not create three separate WRITE_POINTER commands and let the firmware calculate and return the absolute GPAs of the fields F1, F2 and F3. Instead, there will be one WRITE_POINTER command, and QEMU will do the offsetting manually, minimally for fields F2 and F3. "src_offset" looks tempting now only because we have a shared structure with only one field, the GUID at offset 40 decimal. (b) Regarding the runtime addition in the AML code: As discussed before, the main reason *now*, for not pointing VGIA (and other named integer objects) with ADD_POINTER commands directly to "meaningful" fields, is that OVMF probes the targets of ADD_POINTER commands for patterns that look like ACPI table headers. And, for the time being, we want to suppress any mis-recognitions by prepending some padding. Igor was right to dislike this, and we agreed that *down the road* we should add allocation flags, or further allocation commands, to supplant this kind of heuristics in OVMF. But: - we don't have time to do it now, plus - please observe that the runtime addition in AML relates to the ADD_POINTER and the ALLOCATE commands. It does not relate to WRITE_POINTER at all. Whatever we change on WRITE_POINTER will do nothing for suppressing OVMF's table header probing -- because that is tied to ADD_POINTER --, therefore WRITE_POINTER tweaks cannot eliminate the "need to add" in AML. In summary, I think the proposed WRITE_POINTER modification is implementable, but I think it will not pay off, because: (a) for QEMU logic, it will not prove useful as soon as we have a multi-field shared structure (QEMU will have to add field offsets anyway), (b) and for eliminating the AML addition (which is a consequence of the current ADD_POINTER handling in OVMF), it does nothing. Thanks Laszlo