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

Re: [PATCH 8/9] vpci/header: Reset the command register when adding devices

  • To: Jan Beulich <jbeulich@xxxxxxxx>, Oleksandr Andrushchenko <andr2000@xxxxxxxxx>
  • From: Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>
  • Date: Tue, 7 Sep 2021 07:43:24 +0000
  • Accept-language: en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=epam.com; dmarc=pass action=none header.from=epam.com; dkim=pass header.d=epam.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; bh=H05GsjdJnDqyBr0UhP6j4Tyahjtcs2/I/LUt2RtCWRM=; b=UDiDSFZJ2FzFCuCb+IdodVoJGV7fxWUsHvOiJ3ULHlmJKTPdNCaG6IYfs7ASb2zufAfxMqerGFWTVIanLiBaSWP1n44c28m/KjDvRNdaZIqRsjZPUBTQxT7e0+dK8livQOBs29d1vHi62WzIF6hFpU0Qe3qYyTACB0MGT38xyVWJcERFlbzqZqQDL/Av9w67jaFtRyX1rPleknjqW5TBhQ64R5FmMEhsWUmHCdcGCM34qeEbSIdUN8QqjqGP7zD0UmhplSCMK2lZubHbIBYDSwUo8TmqlIgNmt5Ip6KSEeXKmZbUetvXtiiKQq+jCEoe+Q+0fujRwTv4GhwEruB2xA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=HuUhm10ZF3UHBEcJP/PqxNGPc81E9ery6yQihapkOwESeMehOhQdYDMMaK3PpUBnMnFGwPD38p9cgKIDnjlblVr79iu8cSmBDvPfcwQWQZbySbW2D/t4NvrDh9UUfA/ojk2Mz35CTGSQ/qIT7GMztwO8alvq8a0miZKdlbtuhBF62RLXUK/M02xj6bUaD4rq+R67xbhNdELJU9EHDXtqNhAj6yGHvcn6p7SbrcdveHM3NjUiOkw192bpeSOylYsbjHNv4G8FheLOwsWJs4JKcKuPiaY1aAxu96O/ExZ4i1nmcsuBsWNJ8iwwB3Emevy8J1GEc+d+1XkNTUXkHHnnbw==
  • Authentication-results: suse.com; dkim=none (message not signed) header.d=none;suse.com; dmarc=none action=none header.from=epam.com;
  • Cc: "julien@xxxxxxx" <julien@xxxxxxx>, "sstabellini@xxxxxxxxxx" <sstabellini@xxxxxxxxxx>, Oleksandr Tyshchenko <Oleksandr_Tyshchenko@xxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Artem Mygaiev <Artem_Mygaiev@xxxxxxxx>, "roger.pau@xxxxxxxxxx" <roger.pau@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Rahul Singh <rahul.singh@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 07 Sep 2021 07:43:38 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHXoKxjFI8WSqpeJkWkpzZfft/zi6uXHR6AgAEZm4A=
  • Thread-topic: [PATCH 8/9] vpci/header: Reset the command register when adding devices

On 06.09.21 17:55, Jan Beulich wrote:
> On 03.09.2021 12:08, Oleksandr Andrushchenko wrote:
>> --- a/xen/drivers/vpci/header.c
>> +++ b/xen/drivers/vpci/header.c
>> @@ -811,6 +811,16 @@ int vpci_bar_add_handlers(const struct domain *d, 
>> struct pci_dev *pdev)
>>           gprintk(XENLOG_ERR,
>>               "%pp: failed to add BAR handlers for dom%d\n", &pdev->sbdf,
>>               d->domain_id);
>> +
>> +    /*
>> +     * Reset the command register: it is possible that when passing
>> +     * through a PCI device its memory decoding bits in the command
>> +     * register are already set. Thus, a guest OS may not write to the
>> +     * command register to update memory decoding, so guest mappings
>> +     * (guest's view of the BARs) are left not updated.
>> +     */
>> +    pci_conf_write16(pdev->sbdf, PCI_COMMAND, 0);
> Can you really blindly write 0 here? What about bits that have to be
> under host control, e.g. INTX_DISABLE? I can see that you may want to
> hand off with I/O and memory decoding off and bus mastering disabled,
> but for every other bit (including reserved ones) I'd expect separate
> justification (in the commit message).
According to "PCI LOCAL BUS SPECIFICATION, REV. 3.0" I have at hand,
section "6.2.2 Device Control" says that the reset state of the command
register is typically 0, so this is why I chose to write 0 here, e.g.
make the command register as if it is after the reset.

With respect to host control: we currently do not really emulate command
register which probably was ok for x86 PVH Dom0 and this might not be the
case now as we add DomU's. That being said: in my implementation guest can
alter command register as it wants without restrictions.
If we see it does need proper emulation then we would need adding that as
well (is not part of this series though).

Meanwhile, I agree that we can only reset IO space, memory space and bus
master bits and leave the rest untouched. But again, without proper command
register emulation guests can still set what they want.

Please let me know your opinion on how we can proceed.

> Jan
Thank you,




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