Re: [PATCH] xen/acpi: Don't fail if SPCR table is absent

[Julien Grall <julien@xxxxxxx>]

Hi Elliott,

On 22/10/2020 18:13, Elliott Mitchell wrote:
> On Thu, Oct 22, 2020 at 09:42:17AM +0200, Jan Beulich wrote:
> > On 22.10.2020 00:12, Elliott Mitchell wrote:
> > > --- a/xen/arch/arm/acpi/domain_build.c
> > > +++ b/xen/arch/arm/acpi/domain_build.c
> > > @@ -42,17 +42,18 @@ static int __init acpi_iomem_deny_access(struct domain *d)
> > >       status = acpi_get_table(ACPI_SIG_SPCR, 0,
> > >                               (struct acpi_table_header **)&spcr);
> > >   
> > > -    if ( ACPI_FAILURE(status) )
> > > +    if ( ACPI_SUCCESS(status) )
> > >       {
> > > -        printk("Failed to get SPCR table\n");
> > > -        return -EINVAL;
> > > +        mfn = spcr->serial_port.address >> PAGE_SHIFT;
> > > +        /* Deny MMIO access for UART */
> > > +        rc = iomem_deny_access(d, mfn, mfn + 1);
> > > +        if ( rc )
> > > +            return rc;
> > > +    }
> > > +    else
> > > +    {
> > > +        printk("Failed to get SPCR table, Xen console may be unavailable\n");
> > >       }
> > Nit: While I see you've got Stefano's R-b already, I Xen we typically
> > omit the braces here.
> Personally, I prefer that myself, but was unsure of the preference here.
I don't think we are very consistent here... I would not add them 
myself, but I don't particularly mind them (I know some editors will add 
them automatically).
I will keep them while committing. For the patch:

Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

> I've seen multiple projects which *really* dislike using having brackets
> for some clauses, but not others (ie they want either all clauses with or
> all clauses without; instead of only if required).
>
> I sent what I thought was the more often used format.  (I also like tabs,
> and dislike having so many spaces; alas my preferences are apparently
> uncommon)
We have a few files in Xen using tabs (yes, we like mixing coding style!).

Cheers,

--
Julien Grall