[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2] ns16550: enable memory decoding on MMIO-based PCI console card


  • To: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Wed, 26 Apr 2023 10:24:28 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=TxMlZJ8sld0LICCblQtk6hR1iDysMeak36xDyCTXStk=; b=XkVSaZELCE5cbq8+npunXOExfRwxKkXArM7Ek7q3K6BHO1YOOpXdlk5vg6gs3iH5YYJO/YWBHOFyM0y5P8wvkVwQ81j/iynmgxEDakHkQJCL6PlpU/AhFD6bP5aFmhsg+X3CgVjHu1ymZC3DRrpa+Lc7+3SC+SxklJkZlnss0fEQfWwMx+4fNrMU+V1l5Aeay4V+2Yc3ioQSLlh17AK3xwJiQxfxwsHCXTo1SJvAFwCa/Z5YbJbaOWsWrKxaGCoZXzjDMJXTpoCtyOaNTNvevE9J7MlheDJgpLcmrBDTv6awCGsqHunnlQozcQZcK6UOgPLjt2QYq2tgmToEaGoGtg==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ij/HIrcqIBNfVJT5xixCnrTDjwdTIVVqqe6yviN1eJRY5VYOB2CKGJq3ANVf85MR9llI4bnwUQ0m2UoAt0cOtS0cKQONOu2UnKaIay4axssB4lAivnaqcmwoE1bYSZyLwXAHnfTUcknZAyhGt52OVYECpLO0VdcOnWR6XLJkLscbbOezZjGPdhCyVadGb+R1pODmFHQb6bWOsoYjqUAzZjIk2WHeygxxcU0mRZuJXj1so898NIGqxbhrzOK2oslzF/e2imEdtxdfGXmWIvHXpPYc9Wk6l7G3osrldPxsUkww5XruxV0l+aDZXE+0Rh3VsleDsGboG8gsNJ+yFE4Gsw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 26 Apr 2023 08:24:41 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 26.04.2023 09:48, Roger Pau Monné wrote:
> On Tue, Apr 25, 2023 at 04:39:02PM +0200, Marek Marczykowski-Górecki wrote:
>> --- a/xen/drivers/char/ns16550.c
>> +++ b/xen/drivers/char/ns16550.c
>> @@ -272,7 +272,15 @@ static int cf_check ns16550_getc(struct serial_port 
>> *port, char *pc)
>>  static void pci_serial_early_init(struct ns16550 *uart)
>>  {
>>  #ifdef NS16550_PCI
>> -    if ( !uart->ps_bdf_enable || uart->io_base >= 0x10000 )
>> +    if ( uart->bar )
>> +    {
>> +        pci_conf_write16(PCI_SBDF(0, uart->ps_bdf[0], uart->ps_bdf[1],
>> +                                  uart->ps_bdf[2]),
>> +                         PCI_COMMAND, PCI_COMMAND_MEMORY);
> 
> Don't you want to read the current command register first and just
> or PCI_COMMAND_MEMORY?
> 
> I see that for IO decoding we already do it this way, but I'm not sure
> whether it could cause issues down the road by unintentionally
> disabling command flags.

Quite some time ago I asked myself the same question when seeing that
code, but I concluded that perhaps none of the bits are really sensible
to be set for a device as simple as a serial one. I'm actually thinking
that for such a device to be used during early boot, it might even be
helpful if bits like PARITY or SERR get cleared. (Of course if any of
that was really the intention of the change introducing that code, it
should have come with a code comment.)

Jan



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.