Re: [XEN v3 3/9] xen/drivers: ns16550: Use paddr_t for io_base/io_size

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.02.2023 13:05, Ayan Kumar Halder wrote:
> @@ -1166,8 +1166,9 @@ static const struct ns16550_config __initconst 
> uart_config[] =
>  static int __init
>  pci_uart_config(struct ns16550 *uart, bool_t skip_amt, unsigned int idx)
>  {
> -    u64 orig_base = uart->io_base;
> +    paddr_t orig_base = uart->io_base;
>      unsigned int b, d, f, nextf, i;
> +    u64 pci_uart_io_base;

uint64_t please (also elsewhere as needed), assuming the variable
actually needs to survive. And if it needs to, please move it into
a more narrow scope (and perhaps shorten its name).

> @@ -1259,8 +1260,13 @@ pci_uart_config(struct ns16550 *uart, bool_t skip_amt, 
> unsigned int idx)
>                      else
>                          size = len & PCI_BASE_ADDRESS_MEM_MASK;
>  
> -                    uart->io_base = ((u64)bar_64 << 32) |
> -                                    (bar & PCI_BASE_ADDRESS_MEM_MASK);
> +                    pci_uart_io_base = ((u64)bar_64 << 32) |
> +                                        (bar & PCI_BASE_ADDRESS_MEM_MASK);
> +
> +                    /* Truncation detected while converting to paddr_t */
> +                    BUG_ON((pci_uart_io_base >> (PADDR_SHIFT - 1)) > 1);

Why would we want to crash during early boot at all? And then even at a
point where it'll be hard to figure out what's going on, as we're only
in the process of configuring the serial console?

> @@ -1532,7 +1539,12 @@ static bool __init parse_positional(struct ns16550 
> *uart, char **str)
>          else
>  #endif
>          {
> -            uart->io_base = simple_strtoull(conf, &conf, 0);
> +            uart_io_base = simple_strtoull(conf, &conf, 0);
> +
> +            /* Truncation detected while converting to paddr_t */
> +            BUG_ON((uart_io_base >> (PADDR_SHIFT - 1)) > 1);

All the same here.

Jan