Hi Jan,
On 08/02/2023 13:52, Jan Beulich wrote:
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).
Ack.
@@ -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?
I do not understand the best way to return an error from pci_uart_config().
Out of the 4 invocations of pci_uart_config(), the return value is
checked in two invocations only like this.
if ( pci_uart_config(uart, 0, uart - ns16550_com) )
return true;
pci_uart_config() returns 0 in case of success and -1 in case of error.
But the caller seems to be checking the opposite.
I could not use domain_crash() as I understand that pci_uart_config() is
invoked before domain is created.
@@ -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.
I can return false from here and let ns16550_parse_port_config() return.
I think that might be the correct thing to do here.
- Ayan
Jan
