On Sat, 9 Aug 2025 at 22:11, Vadim Chichikalyuk <chichikal...@gmail.com> wrote:
>
> On the ARM virt machine, there is currently no way to programmatically
> discover the frequency of the UART reference clock solely through the use of
> UEFI/ACPI (without the DTB). The SPCR table can include this information
> as of revision 4, but is currently at revision 2.
>
> Bump the revision to 4, add the clock frequency of 24 MHz and
> complete the other new fields.
This commit message should say why we need to do this,
i.e. why we can't continue to do what we've always done
and not tell the guest about a clock frequency which is
entirely imaginary in any case.
> Signed-off-by: Vadim Chichikalyuk <chichikal...@gmail.com>
> ---
> hw/arm/virt-acpi-build.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index b01fc4f8ef..f685668c5e 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -536,6 +536,7 @@ build_iort(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> static void
> spcr_setup(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> {
> + const char name[] = ".";
> AcpiSpcrData serial = {
> .interface_type = 3, /* ARM PL011 UART */
> .base_addr.id = AML_AS_SYSTEM_MEMORY,
> @@ -559,13 +560,14 @@ spcr_setup(GArray *table_data, BIOSLinker *linker,
> VirtMachineState *vms)
> .pci_function = 0,
> .pci_flags = 0,
> .pci_segment = 0,
> + .uart_clk_freq = 24000000,
This is a number which ought to match the existing one in create_fdt()
in hw/arm/virt.c. If we need it in two different files we should
#define it as a constant in include/hw/arm/virt.h.
> + .precise_baudrate = 0,
> + .namespace_string_length = sizeof(name),
> + .namespace_string_offset = 88,
What is this magic number 88 for? The spec says that it's
the offset from the beginning of the structure to wherever
the namespace string is put, but we don't decide that,
the build_spcr() function does. The build_spcr() function
knows what the offset of the name string is, so it should
write this field; the callers shouldn't have to.
> };
thanks
-- PMM
