Re: [PATCH][v3] Pass the location of the ACPI RSDP to DOM0.

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 21/01/2014 20:55, Philip Wernersbach wrote:
> xen: [v3] Pass the location of the ACPI RSDP to DOM0.
>
> Some machines, such as recent IBM servers, only allow the OS to get the
> ACPI RSDP from EFI. Since Xen nukes DOM0's ability to access EFI, DOM0
> cannot get the RSDP on these machines, leading to all sorts of
> functionality reductions.
>
> Signed-off-by: Philip Wernersbach <philip.wernersbach@gmail.com>
>
> ---
> Changed since v2:
>     * Fix coding style
>     * Get rid of extra define
>     * Use correct typedef'd type for the ACPI RSDP pointer
>     * Better error checking conditional
>     * Simplify error message
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index b49256d..fdeb9f2 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1378,6 +1378,25 @@ void __init __start_xen(unsigned long mbi_p)
>              safe_strcat(dom0_cmdline, " acpi=");
>              safe_strcat(dom0_cmdline, acpi_param);
>          }

You have still got incorrect coding style at this point, as indicated in
my previous email.

> +        if ( !strstr(dom0_cmdline, "acpi_rsdp=") )
> +        {
> +            acpi_physical_address rp = acpi_os_get_root_pointer();
> +            char rp_str[sizeof(acpi_physical_address)*2 + 1];
> +
> +            if ( rp )
> +            {
> +                snprintf(rp_str, sizeof(acpi_physical_address)*2 + 1,

sizeof(rp_str)

> +                         "%08lX", rp);

Personally, I prefer lowercase hexidecimal numbers, as they are easier
to read, particularly when 64bit.  What happens if the root pointer is
above the 4GB boundary? I dont see any reason at all for the leading 0s.

> +
> +                safe_strcat(dom0_cmdline, " acpi_rsdp=0x");
> +                safe_strcat(dom0_cmdline, rp_str);
> +            }
> +            else
> +            {

And coding style here.

> +                printk(XENLOG_WARNING
> +                       "Failed to get acpi_rsdp to pass to dom0\n");
> +            }
> +        }

And finally, you have yet to address Jan's concerns about this patch. 
Being an x86 maintainer, it is him you will have to convince to accept
the patch, even after I have run out of basic review points to cover.

~Andrew