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

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

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 ==
>  
>

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

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

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