All of lore.kernel.org
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	edk2-devel@lists.01.org
Cc: leif.lindholm@linaro.org, jordan.l.justen@intel.com,
	Shannon Zhao <zhaoshenglong@huawei.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Igor Mammedov <imammedo@redhat.com>,
	qemu devel list <qemu-devel@nongnu.org>,
	gengdongjiu <gengdongjiu@huawei.com>,
	Drew Jones <drjones@redhat.com>
Subject: Re: [Qemu-devel] [RFC PATCH] OvmfPkg/AcpiPlatformDxe: lift 4 GB alloc limit for modern ACPI systems
Date: Sat, 3 Jun 2017 00:40:20 +0200	[thread overview]
Message-ID: <eb98217b-5559-51a0-0e42-559b85b5fba7@redhat.com> (raw)
In-Reply-To: <1496415382.16895.4.camel@redhat.com>

On 06/02/17 16:56, Gerd Hoffmann wrote:
>   Hi,
> 
>>  The reason is that old firmware is allowed to fail on new QEMU
>> (regardless of machine type). Example: the WRITE_POINTER command,
>> originally introduced for VMGENID. If you run a SeaBIOS binary
>> without
>> WRITE_POINTER support, in a QEMU VM with "-device vmgenid", the
>> device
>> will not work.
> 
> Old seabios will work just fine on new qemu as long as you don't use
> the vmgenid device.
> 
> Only when using new features (which require firmware support) new
> seabios is needed, and usually we update seabios for that reason.  We
> want the new features be usable of course.  But in general there are no
> lockstep updates for qemu and seabios (any more).
> 
>> And this patch is actually that case, because the new fw would
>> allocate
>> blobs with such 8-byte addresses that might not fit into 32-bit blob
>> fields.
> 
> I think ovmf simply checking whenever the pointer fields are 32bit or
> 64bit, then doing allocations accordingly (as Igor suggested) would be
> the best.  Why ask qemu for a hint when ovmf can figure on its own?
> That adds a new interface and new config knobs for IMO no good reason.

There is a good reason: the reason is that OVMF should not be forced to
reverse-engineer such information with a third, separate pass over the
linker/loader script, adding yet more complexity.

The linker/loader interface was designed for SeaBIOS, without any regard
to OVMF / ArmVirtQemu. For example, SeaBIOS gets hints like HIGH vs.
FSEG (which distinction is irrelevant for OVMF / ArmVirtQemu) despite
the fact that SeaBIOS could also figure that out one way or another if
it really wanted to. OVMF / ArmVirtQemu need different kinds of hints,
and I think they should get them at this point.

Even now, OVMF deduces the starts of individual ACPI tables in the blobs
by using somewhat messy heuristics. (OVMF *must* know where the tables
start.) The heuristics incur no false negatives, but a chance exists for
false positives, unless QEMU takes extra precautions. But Igor happens
to hate those precautions, and the TCPA and NVDIMM authors have
completely missed them. So we should have fewer of these heuristics and
workarounds, not more.

And, as I replied to Igor earlier, the other blob allocation hint (the
"no ACPI content" hint that allows QEMU to drop precisely the above
obscure precautions, i.e. the "ACPI SDT header probe suppressor"),
necessitates extending the ALLOCATE command anyway.

Would it help if I made these extensions dependent on 2.10+ machine
types? (Obviously existing generators like TCPA, NVDIMM and VMGENID
would not be modified then, because the old stuff would have to be
preserved for old machine types anyway.)

Thanks
Laszlo

      reply	other threads:[~2017-06-02 22:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20170601112241.2580-1-ard.biesheuvel@linaro.org>
2017-06-01 12:25 ` [Qemu-devel] [RFC PATCH] OvmfPkg/AcpiPlatformDxe: lift 4 GB alloc limit for modern ACPI systems Laszlo Ersek
2017-06-01 15:16   ` Igor Mammedov
2017-06-01 16:37     ` Laszlo Ersek
2017-06-01 20:40   ` Laszlo Ersek
2017-06-02 14:56     ` Gerd Hoffmann
2017-06-02 22:40       ` Laszlo Ersek [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=eb98217b-5559-51a0-0e42-559b85b5fba7@redhat.com \
    --to=lersek@redhat.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=drjones@redhat.com \
    --cc=edk2-devel@lists.01.org \
    --cc=gengdongjiu@huawei.com \
    --cc=imammedo@redhat.com \
    --cc=jordan.l.justen@intel.com \
    --cc=kraxel@redhat.com \
    --cc=leif.lindholm@linaro.org \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=zhaoshenglong@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.