* [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size
@ 2023-09-12 15:55 Laszlo Ersek
2023-09-12 16:40 ` Philippe Mathieu-Daudé
2023-09-12 17:30 ` Michael Tokarev
0 siblings, 2 replies; 4+ messages in thread
From: Laszlo Ersek @ 2023-09-12 15:55 UTC (permalink / raw)
To: qemu-devel, lersek
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Paolo Bonzini,
Richard Henderson, Eduardo Habkost, qemu-trivial
- The comment is incorrectly indented / formatted.
- The comment states a 8MB limit, even though the code enforces a 16MB
limit.
Both of these warts come from commit 0657c657eb37 ("hw/i386/pc: add max
combined fw size as machine configuration option", 2020-12-09); clean them
up.
Arguably, it's also better to be consistent with the binary units (such as
"MiB") that QEMU uses nowadays.
Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:PC)
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> (supporter:PC)
Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86 TCG CPUs)
Cc: Richard Henderson <richard.henderson@linaro.org> (maintainer:X86 TCG CPUs)
Cc: Eduardo Habkost <eduardo@habkost.net> (maintainer:X86 TCG CPUs)
Cc: qemu-trivial@nongnu.org
Fixes: 0657c657eb37
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
Notes:
v2:
- use the binary units MiB, KiB, GiB comprehensively in the comment
hw/i386/pc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 54838c0c411d..0b642e8af590 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1746,12 +1746,12 @@ static void pc_machine_set_max_fw_size(Object *obj, Visitor *v,
}
/*
- * We don't have a theoretically justifiable exact lower bound on the base
- * address of any flash mapping. In practice, the IO-APIC MMIO range is
- * [0xFEE00000..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free
- * only 18MB-4KB below 4G. For now, restrict the cumulative mapping to 8MB in
- * size.
- */
+ * We don't have a theoretically justifiable exact lower bound on the base
+ * address of any flash mapping. In practice, the IO-APIC MMIO range is
+ * [0xFEE00000..0xFEE01000] -- see IO_APIC_DEFAULT_ADDRESS --, leaving free
+ * only 18MiB-4KiB below 4GiB. For now, restrict the cumulative mapping to
+ * 16MiB in size.
+ */
if (value > 16 * MiB) {
error_setg(errp,
"User specified max allowed firmware size %" PRIu64 " is "
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size
2023-09-12 15:55 [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size Laszlo Ersek
@ 2023-09-12 16:40 ` Philippe Mathieu-Daudé
2023-09-13 11:02 ` Laszlo Ersek
2023-09-12 17:30 ` Michael Tokarev
1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-09-12 16:40 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Paolo Bonzini,
Richard Henderson, Eduardo Habkost, qemu-trivial
On 12/9/23 17:55, Laszlo Ersek wrote:
> - The comment is incorrectly indented / formatted.
>
> - The comment states a 8MB limit, even though the code enforces a 16MB
> limit.
>
> Both of these warts come from commit 0657c657eb37 ("hw/i386/pc: add max
> combined fw size as machine configuration option", 2020-12-09); clean them
> up.
>
> Arguably, it's also better to be consistent with the binary units (such as
> "MiB") that QEMU uses nowadays.
>
> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:PC)
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> (supporter:PC)
> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86 TCG CPUs)
> Cc: Richard Henderson <richard.henderson@linaro.org> (maintainer:X86 TCG CPUs)
> Cc: Eduardo Habkost <eduardo@habkost.net> (maintainer:X86 TCG CPUs)
> Cc: qemu-trivial@nongnu.org
> Fixes: 0657c657eb37
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>
> Notes:
> v2:
>
> - use the binary units MiB, KiB, GiB comprehensively in the comment
I was going to suggest that ;)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> hw/i386/pc.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size
2023-09-12 15:55 [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size Laszlo Ersek
2023-09-12 16:40 ` Philippe Mathieu-Daudé
@ 2023-09-12 17:30 ` Michael Tokarev
1 sibling, 0 replies; 4+ messages in thread
From: Michael Tokarev @ 2023-09-12 17:30 UTC (permalink / raw)
To: Laszlo Ersek, qemu-devel
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Paolo Bonzini,
Richard Henderson, Eduardo Habkost, qemu-trivial
12.09.2023 18:55, Laszlo Ersek:
> - The comment is incorrectly indented / formatted.
>
> - The comment states a 8MB limit, even though the code enforces a 16MB
> limit.
>
> Both of these warts come from commit 0657c657eb37 ("hw/i386/pc: add max
> combined fw size as machine configuration option", 2020-12-09); clean them
> up.
>
> Arguably, it's also better to be consistent with the binary units (such as
> "MiB") that QEMU uses nowadays.
Applied to my trivial-patches tree. Thanks!
/mjt
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size
2023-09-12 16:40 ` Philippe Mathieu-Daudé
@ 2023-09-13 11:02 ` Laszlo Ersek
0 siblings, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2023-09-13 11:02 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Paolo Bonzini,
Richard Henderson, Eduardo Habkost, qemu-trivial
On 9/12/23 18:40, Philippe Mathieu-Daudé wrote:
> On 12/9/23 17:55, Laszlo Ersek wrote:
>> - The comment is incorrectly indented / formatted.
>>
>> - The comment states a 8MB limit, even though the code enforces a 16MB
>> limit.
>>
>> Both of these warts come from commit 0657c657eb37 ("hw/i386/pc: add max
>> combined fw size as machine configuration option", 2020-12-09); clean
>> them
>> up.
>>
>> Arguably, it's also better to be consistent with the binary units
>> (such as
>> "MiB") that QEMU uses nowadays.
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com> (supporter:PC)
>> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> (supporter:PC)
>> Cc: Paolo Bonzini <pbonzini@redhat.com> (maintainer:X86 TCG CPUs)
>> Cc: Richard Henderson <richard.henderson@linaro.org> (maintainer:X86
>> TCG CPUs)
>> Cc: Eduardo Habkost <eduardo@habkost.net> (maintainer:X86 TCG CPUs)
>> Cc: qemu-trivial@nongnu.org
>> Fixes: 0657c657eb37
>> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
>> ---
>>
>> Notes:
>> v2:
>> - use the binary units MiB, KiB, GiB comprehensively in the
>> comment
>
> I was going to suggest that ;)
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
And when I was writing the patch, I was 100% sure that you were going to
be my first reviewer. :)
Thanks!
Laszlo
>
>>
>> hw/i386/pc.c | 12 ++++++------
>> 1 file changed, 6 insertions(+), 6 deletions(-)
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-09-13 11:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-12 15:55 [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size Laszlo Ersek
2023-09-12 16:40 ` Philippe Mathieu-Daudé
2023-09-13 11:02 ` Laszlo Ersek
2023-09-12 17:30 ` Michael Tokarev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).