[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: Thu, 9 Sep 2021 08:50:33 +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=iqLuCiWGA7vzMyytONa+qXpS50N6PCZ5JTuyAmXc0Ec=; b=dGWeeWs6sxQrJDVqqSfbgTRoprhCZVJZwDAuRZ2Bvvz31EiMY+JmSt14sdmi6LovG0NoQBNGRF1t92825T3DOW4H/7RoW+PAAplpwLJjw+37OtPTj9fF788Ozxt4jzmChxErt38A1xYe4XFqfgxOAuBK2lRurqwbVzowLkxsOC0K8WK2L4l3ENlUzAFvNWoS/ii4m5lOcfShVACL0C7qq0MsBVUIrf/e/djby9HAKGVs+mgPfjS2nino98oeXgtLQccwJU5dxafdV35Ys1DfArMj3mS0hkJ/9eUoE1UMHyIBIlInvSMJRSNjqzv1lRCpqUEU0jvF1yGgZ9hFUa5CGA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bvIoCU1C4d9RnGGqwH8u1CwTSHxaVbB8PB53MTqF67Ix3+Cet1D0Rb2EOwPGolMOvdwmq+oEjTxbm/zH++Gda/Cx/6mpt51A2Kn9/eBhtLc9IfqUClI8fAkoKf5Jrd7c3eLml/ixebDY+0hffHD/m6F45YfF5xljOZW/2K2Sb5M2IDQe3Dg3xLgwA5Q+g3JdVXeK0I6tetYFvGTW/sQGlCav324cNn9enKC6toq/i+vUqFeDYKwzvvXqz1IG+2NBd+spK3O/+Xmugfa99ujdYaolC46o5isWUuTwO6ZhkyxqFOx8dQULLFIbQ7By3oPupUBbtcHeWI+Rq9fRELVi8g==
  • 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: Thu, 09 Sep 2021 08:50:48 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-topic: [PATCH 8/9] vpci/header: Reset the command register when adding devices

On 09.09.21 11:43, Jan Beulich wrote:
> On 09.09.2021 10:39, Oleksandr Andrushchenko wrote:
>> On 07.09.21 13:06, Jan Beulich wrote:
>>> On 07.09.2021 11:52, Oleksandr Andrushchenko wrote:
>>>> On 07.09.21 12:19, Jan Beulich wrote:
>>>>> On 07.09.2021 11:07, Oleksandr Andrushchenko wrote:
>>>>>> On 07.09.21 11:49, Jan Beulich wrote:
>>>>>>> On 07.09.2021 10:18, Oleksandr Andrushchenko wrote:
>>>>>>>> So, if we have a hidden PCI device which can be assigned to a guest 
>>>>>>>> and it is literally untouched
>>>>>>>> (not enabled in Dom0) then I think there will be no such reference as 
>>>>>>>> "host assigned values" as
>>>>>>>> most probably the command register will remain in its after reset 
>>>>>>>> state.
>>>>>>> What meaning of "hidden" do you imply here? Devices passed to
>>>>>>> pci_{hide,ro}_device() may not be assigned to guests ...
>>>>>> You are completely right here.
>>>>>>> For any other meaning of "hidden", even if the device is completely
>>>>>>> ignored by Dom0,
>>>>>> Dom0less is such a case when a device is assigned to the guest
>>>>>> without Dom0 at all?
>>>>> In this case it is entirely unclear to me what entity it is to have
>>>>> a global view on the PCI subsystem.
>>>>>>>      certain of the properties still cannot be allowed
>>>>>>> to be DomU-controlled.
>>>>>> The list is not that big, could you please name a few you think cannot
>>>>>> be controlled by a guest? I can think of PCI_COMMAND_SPECIAL(?),
>>>>>> PCI_COMMAND_SERR, PCI_COMMAND_INTX_DISABLE which we may want to
>>>>>> be aligned with the "host reference" values, e.g. we only allow those 
>>>>>> bits
>>>>>> to be set as they are in Dom0.
>>>>> Well, you've compile a list already, and I did say so before as well:
>>>>> Everything except I/O and memory decoding as well as bus mastering
>>>>> needs at least closely looking at. INTX_DISABLE, for example, is
>>>>> something I don't think a guest should be able to directly control.
>>>>> It may still be the case that the host permits it control, but then
>>>>> only indirectly, allowing the host to appropriately adjust its
>>>>> internals.
>>>>> Note that even for I/O and memory decoding as well as bus mastering
>>>>> it may be necessary to limit guest control: In case the host wants
>>>>> to disable any of these (perhaps transiently) despite the guest
>>>>> wanting them enabled.
>>>> Ok, so it is now clear that we need a yet another patch to add a proper
>>>> command register emulation. What is your preference: drop the current
>>>> patch, implement command register emulation and add a "reset patch"
>>>> after that or we can have the patch as is now, but I'll only reset IO/mem 
>>>> and bus
>>>> master bits, e.g. read the real value, mask the wanted bits and write back?
>>> Either order is fine with me as long as the result will be claimed to
>>> be complete until proper emulation is in place.
>> I tried to see what others do in order to emulate PCI_COMMAND register
>> and it seems that at most they care about the only INTX bit (besides
>> IO/memory enable and bus muster which are write through). Please see
>> [1] and [2]. Probably I miss something, but it could be because in order
>> to properly emulate the COMMAND register we need to know about the
>> whole PCI topology, e.g. if any setting in device's command register
>> is aligned with the upstream port etc. This makes me think that because
>> of this complexity others just ignore that. Neither I think this can be
>> easily done in our case. So I would suggest we just add the following
>> simple logic to only emulate PCI_COMMAND_INTX_DISABLE: allow guest to
>> disable the interrupts, but don't allow to enable if host has disabled
>> them. This is also could be tricky a bit for the devices which are not
>> enabled and thus not configured in Dom0, e.g. we do not know for sure
>> if the value in the PCI_COMMAND register (in particular
>> PCI_COMMAND_INTX_DISABLE bit) can be used as the reference host value or
>> not. It can be that the value there is just the one after reset or so.
>> The rest of the command register bits will go directly to the command
>> register untouched.
>> So, at the end of the day the question is if PCI_COMMAND_INTX_DISABLE
>> is enough and how to get its reference host value.
> Well, in order for the whole thing to be security supported it needs to
> be explained for every bit why it is safe to allow the guest to drive it.

So, do we want at least PCI_COMMAND_INTX_DISABLE bit aligned

between the host and guest? If so, what do you you think about

the reference value for it (please see above).

> Until you mean vPCI to reach that state, leaving TODO notes in the code
> for anything not investigated may indeed be good enough.
Ok, I'll add TODO then.
> Jan
Thank you,




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