[PATCH v2] hw/i386/pc: fix code comment on cumulative flash size

Laszlo Ersek posted 1 patch 7 months, 2 weeks ago
Failed in applying to current master (apply log)
hw/i386/pc.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
[PATCH v2] hw/i386/pc: fix code comment on cumulative flash size
Posted by Laszlo Ersek 7 months, 2 weeks ago
- 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 "
Re: [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size
Posted by Michael Tokarev 7 months, 2 weeks ago
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
Re: [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size
Posted by Philippe Mathieu-Daudé 7 months, 2 weeks ago
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(-)


Re: [PATCH v2] hw/i386/pc: fix code comment on cumulative flash size
Posted by Laszlo Ersek 7 months, 2 weeks ago
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(-)
>