[Qemu-devel] [PATCH v2 0/2] fw_cfg: documentation updates for v2.4

[Qemu-devel] [PATCH v2 0/2] fw_cfg: documentation updates for v2.4

[Gabriel L. Somlo]

Patch 1/2 is a re-submission of an earlier v1 which may have fallen
through the cracks (or gotten accidentally stuck with semi-unrelated
code-change RFC patches). It documents a fix commited at
48779e501810c5046ff8af7b9cf9c99bec2928a1

Patch 2/2 documents the current MMIO register addresses on arm, sun4m,
and ppc/mac.

Please consider applying before the final 2.4 release.

Thanks,
  --Gabriel

Gabriel L. Somlo (2):
  fw_cfg: document fw_cfg_modify_iXX() update functions
  fw_cfg: document MMIO registers for arm, sun4*, and ppc/mac_*

 docs/specs/fw_cfg.txt | 33 ++++++++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

-- 
2.1.0

[Qemu-devel] [PATCH v2 1/2] fw_cfg: document fw_cfg_modify_iXX() update functions

[Gabriel L. Somlo]

Document the behavior of fw_cfg_modify_iXX() for leak-free update
of integer fw_cfg blobs; this function was added as a bug fix with
commit 48779e501810c5046ff8af7b9cf9c99bec2928a1.

Currently only fw_cfg_modify_i16() is coded, but 32- and 64-bit
versions may be added later if necessary.

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 docs/specs/fw_cfg.txt | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 74351dd..5bc7b96 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -159,6 +159,17 @@ will convert a 16-, 32-, or 64-bit integer to little-endian, then add
 a dynamically allocated copy of the appropriately sized item to fw_cfg
 under the given selector key value.
 
+== fw_cfg_modify_iXX() ==
+
+Modify the value of an XX-bit item (where XX may be 16, 32, or 64).
+Similarly to the corresponding fw_cfg_add_iXX() function set, convert
+a 16-, 32-, or 64-bit integer to little endian, create a dynamically
+allocated copy of the required size, and replace the existing item at
+the given selector key value with the newly allocated one. The previous
+item, assumed to have been allocated during an earlier call to
+fw_cfg_add_iXX() or fw_cfg_modify_iXX() (of the same width XX), is freed
+before the function returns.
+
 == fw_cfg_add_file() ==
 
 Given a filename (i.e., fw_cfg item name), starting pointer, and size,
-- 
2.1.0

[Qemu-devel] [PATCH v2 2/2] fw_cfg: document MMIO registers for arm, sun4*, and ppc/mac_*

[Gabriel L. Somlo]

Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
---
 docs/specs/fw_cfg.txt | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 5bc7b96..f37e1ad 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -72,10 +72,26 @@ increasing address order, similar to memcpy().
 
 == Register Locations ==
 
-=== x86, x86_64 Register Locations ===
+=== x86, x86_64, sun4u Register Locations ===
+
+Selector Register IOport: 0x510 (16-bit, little-endian)
+Data Register IOport:     0x511 (8-bit)
+
+=== arm Register Locations ===
+
+Selector Register MMIO addr: 0x9020008 (16-bit, big-endian)
+Data Register MMIO addr:     0x9020000 (64-bit)
+
+=== sun4m Register Locations ===
+
+Selector Register MMIO addr: 0xd00000510 (16-bit, big-endian)
+Data Register MMIO addr:     0xd00000512 (8-bit)
+
+=== ppc/mac Register Locations ===
+
+Selector Register MMIO addr: 0xf0000510 (16-bit, big-endian)
+Data Register MMIO addr:     0xf0000512 (8-bit)
 
-Selector Register IOport: 0x510
-Data Register IOport:     0x511
 
 == Firmware Configuration Items ==
 
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2 0/2] fw_cfg: documentation updates for v2.4

[Peter Maydell]

On 6 August 2015 at 00:47, Gabriel L. Somlo <somlo@cmu.edu> wrote:
> Patch 1/2 is a re-submission of an earlier v1 which may have fallen
> through the cracks (or gotten accidentally stuck with semi-unrelated
> code-change RFC patches). It documents a fix commited at
> 48779e501810c5046ff8af7b9cf9c99bec2928a1
>
> Patch 2/2 documents the current MMIO register addresses on arm, sun4m,
> and ppc/mac.
>
> Please consider applying before the final 2.4 release.

Not unless we absolutely have to have another rc, I'm afraid:
you have missed the boat...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 1/2] fw_cfg: document fw_cfg_modify_iXX() update functions

[Laszlo Ersek]

On 08/06/15 01:47, Gabriel L. Somlo wrote:
> Document the behavior of fw_cfg_modify_iXX() for leak-free update
> of integer fw_cfg blobs; this function was added as a bug fix with
> commit 48779e501810c5046ff8af7b9cf9c99bec2928a1.
> 
> Currently only fw_cfg_modify_i16() is coded, but 32- and 64-bit
> versions may be added later if necessary.
> 
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> ---
>  docs/specs/fw_cfg.txt | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 74351dd..5bc7b96 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -159,6 +159,17 @@ will convert a 16-, 32-, or 64-bit integer to little-endian, then add
>  a dynamically allocated copy of the appropriately sized item to fw_cfg
>  under the given selector key value.
>  
> +== fw_cfg_modify_iXX() ==
> +
> +Modify the value of an XX-bit item (where XX may be 16, 32, or 64).
> +Similarly to the corresponding fw_cfg_add_iXX() function set, convert
> +a 16-, 32-, or 64-bit integer to little endian, create a dynamically
> +allocated copy of the required size, and replace the existing item at
> +the given selector key value with the newly allocated one. The previous
> +item, assumed to have been allocated during an earlier call to
> +fw_cfg_add_iXX() or fw_cfg_modify_iXX() (of the same width XX), is freed
> +before the function returns.
> +
>  == fw_cfg_add_file() ==
>  
>  Given a filename (i.e., fw_cfg item name), starting pointer, and size,
> 

I think I gave my R-b for this here:

http://thread.gmane.org/gmane.comp.emulators.qemu/352278/focus=352299

Re: [Qemu-devel] [PATCH v2 2/2] fw_cfg: document MMIO registers for arm, sun4*, and ppc/mac_*

[Laszlo Ersek]

On 08/06/15 01:47, Gabriel L. Somlo wrote:
> Signed-off-by: Gabriel Somlo <somlo@cmu.edu>
> ---
>  docs/specs/fw_cfg.txt | 22 +++++++++++++++++++---
>  1 file changed, 19 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
> index 5bc7b96..f37e1ad 100644
> --- a/docs/specs/fw_cfg.txt
> +++ b/docs/specs/fw_cfg.txt
> @@ -72,10 +72,26 @@ increasing address order, similar to memcpy().
>  
>  == Register Locations ==
>  
> -=== x86, x86_64 Register Locations ===
> +=== x86, x86_64, sun4u Register Locations ===
> +
> +Selector Register IOport: 0x510 (16-bit, little-endian)
> +Data Register IOport:     0x511 (8-bit)
> +
> +=== arm Register Locations ===
> +
> +Selector Register MMIO addr: 0x9020008 (16-bit, big-endian)
> +Data Register MMIO addr:     0x9020000 (64-bit)

Suggestions:
- maybe mention that this is specific to the "virt" machtype
- mention that the exact location comes from the DTB,
  and hint at "Documentation/devicetree/bindings/arm/fw-cfg.txt" in the
  kernel tree
- for the data register, rather than just "(64-bit)", consider saying
  "(64-bit, endianless, string-preserving)"

> +
> +=== sun4m Register Locations ===
> +
> +Selector Register MMIO addr: 0xd00000510 (16-bit, big-endian)
> +Data Register MMIO addr:     0xd00000512 (8-bit)
> +
> +=== ppc/mac Register Locations ===
> +
> +Selector Register MMIO addr: 0xf0000510 (16-bit, big-endian)
> +Data Register MMIO addr:     0xf0000512 (8-bit)

Should these be tied to machine types? (I got no clue, I've never even
built these targets.)

Thanks
Laszlo

>  
> -Selector Register IOport: 0x510
> -Data Register IOport:     0x511
>  
>  == Firmware Configuration Items ==
>  
> 

Re: [Qemu-devel] [PATCH v2 2/2] fw_cfg: document MMIO registers for arm, sun4*, and ppc/mac_*

[Peter Maydell]

On 6 August 2015 at 10:14, Laszlo Ersek <lersek@redhat.com> wrote:
> On 08/06/15 01:47, Gabriel L. Somlo wrote:

>> +=== arm Register Locations ===
>> +
>> +Selector Register MMIO addr: 0x9020008 (16-bit, big-endian)
>> +Data Register MMIO addr:     0x9020000 (64-bit)
>
> Suggestions:
> - maybe mention that this is specific to the "virt" machtype
> - mention that the exact location comes from the DTB,
>   and hint at "Documentation/devicetree/bindings/arm/fw-cfg.txt" in the
>   kernel tree
> - for the data register, rather than just "(64-bit)", consider saying
>   "(64-bit, endianless, string-preserving)"

We shouldn't be documenting the specific locations at all --
the guest *must* look at the DTB or ACPI table to find the
device.

-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] fw_cfg: document MMIO registers for arm, sun4*, and ppc/mac_*

[Gabriel L. Somlo]

On Thu, Aug 06, 2015 at 02:15:11PM +0100, Peter Maydell wrote:
> On 6 August 2015 at 10:14, Laszlo Ersek <lersek@redhat.com> wrote:
> > On 08/06/15 01:47, Gabriel L. Somlo wrote:
> 
> >> +=== arm Register Locations ===
> >> +
> >> +Selector Register MMIO addr: 0x9020008 (16-bit, big-endian)
> >> +Data Register MMIO addr:     0x9020000 (64-bit)
> >
> > Suggestions:
> > - maybe mention that this is specific to the "virt" machtype
> > - mention that the exact location comes from the DTB,
> >   and hint at "Documentation/devicetree/bindings/arm/fw-cfg.txt" in the
> >   kernel tree
> > - for the data register, rather than just "(64-bit)", consider saying
> >   "(64-bit, endianless, string-preserving)"
> 
> We shouldn't be documenting the specific locations at all --
> the guest *must* look at the DTB or ACPI table to find the
> device.

I see how that may work on x86 (acpi) or arm (dtb). But would either of
those also work for sun4 and ppc/mac ? Do we even care ? ;)

Thanks,
--Gabriel

Re: [Qemu-devel] [PATCH v2 2/2] fw_cfg: document MMIO registers for arm, sun4*, and ppc/mac_*

[Peter Maydell]

On 6 August 2015 at 15:49, Gabriel L. Somlo <somlo@cmu.edu> wrote:
> On Thu, Aug 06, 2015 at 02:15:11PM +0100, Peter Maydell wrote:
>> On 6 August 2015 at 10:14, Laszlo Ersek <lersek@redhat.com> wrote:
>> > On 08/06/15 01:47, Gabriel L. Somlo wrote:
>>
>> >> +=== arm Register Locations ===
>> >> +
>> >> +Selector Register MMIO addr: 0x9020008 (16-bit, big-endian)
>> >> +Data Register MMIO addr:     0x9020000 (64-bit)
>> >
>> > Suggestions:
>> > - maybe mention that this is specific to the "virt" machtype
>> > - mention that the exact location comes from the DTB,
>> >   and hint at "Documentation/devicetree/bindings/arm/fw-cfg.txt" in the
>> >   kernel tree
>> > - for the data register, rather than just "(64-bit)", consider saying
>> >   "(64-bit, endianless, string-preserving)"
>>
>> We shouldn't be documenting the specific locations at all --
>> the guest *must* look at the DTB or ACPI table to find the
>> device.
>
> I see how that may work on x86 (acpi) or arm (dtb). But would either of
> those also work for sun4 and ppc/mac ? Do we even care ? ;)

That comment was specifically about ARM, which is why I only
quoted the bit of your patch relating to ARM. I don't know
how ppc and sparc are currently locating the fw_cfg thing.

-- PMM