All of lore.kernel.org
 help / color / mirror / Atom feed
* [virtio-comment] virtio member device provisioning get/set and virtio child device life cycle mixing not needed
@ 2023-06-28  0:22 Parav Pandit
  2023-06-28  7:30 ` [virtio-comment] " Zhu, Lingshan
  0 siblings, 1 reply; 12+ messages in thread
From: Parav Pandit @ 2023-06-28  0:22 UTC (permalink / raw)
  To: lingshan.zhu; +Cc: virtio-comment, Xuan Zhuo, virtio

Hi Lingshan Zhu,

I have reviewed v4 version transport VQ a while back that consist of two main pieces.

1. some virtio device creation/destroy commands
2. virtio device feature and config space provisioning commands.

Many of us are seeing the value of device provisioning commands with/without live migration.

New virtio devices do not need to follow any existing of config space etc per say for real hw.
There is some value in reusing software but it may not be the prime driving factor for brand new devices.
This needs more discussion and not as_is done in the v4.

So, if you are planning to revise your transport vq patches as admin commands, please split the series into two.

a. device and config space provisioning SET/GET part.
b. virtio child device creation aka SIOV/SF etc.

First #a, followed by #b.
Piece #a will be usable for SRIOV VFs + SF/SIOV devices and provides unified APIs across two different device group types.

Parav

This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-28  0:22 [virtio-comment] virtio member device provisioning get/set and virtio child device life cycle mixing not needed Parav Pandit
@ 2023-06-28  7:30 ` Zhu, Lingshan
  2023-06-28 13:10   ` Parav Pandit
  0 siblings, 1 reply; 12+ messages in thread
From: Zhu, Lingshan @ 2023-06-28  7:30 UTC (permalink / raw)
  To: Parav Pandit; +Cc: virtio-comment, Xuan Zhuo, virtio



On 6/28/2023 8:22 AM, Parav Pandit wrote:
> Hi Lingshan Zhu,
>
> I have reviewed v4 version transport VQ a while back that consist of two main pieces.
>
> 1. some virtio device creation/destroy commands
> 2. virtio device feature and config space provisioning commands.
>
> Many of us are seeing the value of device provisioning commands with/without live migration.
>
> New virtio devices do not need to follow any existing of config space etc per say for real hw.
> There is some value in reusing software but it may not be the prime driving factor for brand new devices.
> This needs more discussion and not as_is done in the v4.
>
> So, if you are planning to revise your transport vq patches as admin commands, please split the series into two.
>
> a. device and config space provisioning SET/GET part.
Hi Parav

I agree it is good to unify the interface for SRIOV and SIOV. However 
more discussion is required,
e.g., for a VF, will config its MSIX by admin vq(owned by host) conflict 
with the MSIX cap.

Thanks,
Zhu Lingshan
> b. virtio child device creation aka SIOV/SF etc.
>
> First #a, followed by #b.
> Piece #a will be usable for SRIOV VFs + SF/SIOV devices and provides unified APIs across two different device group types.
>
> Parav


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-28  7:30 ` [virtio-comment] " Zhu, Lingshan
@ 2023-06-28 13:10   ` Parav Pandit
  2023-06-29  2:18     ` Zhu, Lingshan
  0 siblings, 1 reply; 12+ messages in thread
From: Parav Pandit @ 2023-06-28 13:10 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: virtio-comment, Xuan Zhuo, virtio



> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
> open.org> On Behalf Of Zhu, Lingshan
> Sent: Wednesday, June 28, 2023 3:31 AM
> 
> On 6/28/2023 8:22 AM, Parav Pandit wrote:
> > Hi Lingshan Zhu,
> >
> > I have reviewed v4 version transport VQ a while back that consist of two main
> pieces.
> >
> > 1. some virtio device creation/destroy commands 2. virtio device
> > feature and config space provisioning commands.
> >
> > Many of us are seeing the value of device provisioning commands
> with/without live migration.
> >
> > New virtio devices do not need to follow any existing of config space etc per
> say for real hw.
> > There is some value in reusing software but it may not be the prime driving
> factor for brand new devices.
> > This needs more discussion and not as_is done in the v4.
> >
> > So, if you are planning to revise your transport vq patches as admin
> commands, please split the series into two.
> >
> > a. device and config space provisioning SET/GET part.
> Hi Parav
> 
> I agree it is good to unify the interface for SRIOV and SIOV. However more
> discussion is required, e.g., for a VF, will config its MSIX by admin vq(owned by
> host) conflict with the MSIX cap.
> 
No, it doesn't conflict.
SR-PCIM interface is outside the scope of PCIe spec.
I completed the work in PCI spec to clarify this already, its coming in the 6.1 errata spec.

Non virtio device and Linux kernel both already has similar implementation in place and in use for more than a year. 
So msix provisioning via AQ is fine. VF can expose new MSIX capability post reconfiguration via AQ.

We will be able to unify them across SRIOV and SIOV.
SIOV anyway has to wait as its undergoing significant changes currently.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-28 13:10   ` Parav Pandit
@ 2023-06-29  2:18     ` Zhu, Lingshan
  2023-06-29  2:23       ` Parav Pandit
  0 siblings, 1 reply; 12+ messages in thread
From: Zhu, Lingshan @ 2023-06-29  2:18 UTC (permalink / raw)
  To: Parav Pandit; +Cc: virtio-comment, Xuan Zhuo, virtio



On 6/28/2023 9:10 PM, Parav Pandit wrote:
>
>> From: virtio-comment@lists.oasis-open.org <virtio-comment@lists.oasis-
>> open.org> On Behalf Of Zhu, Lingshan
>> Sent: Wednesday, June 28, 2023 3:31 AM
>>
>> On 6/28/2023 8:22 AM, Parav Pandit wrote:
>>> Hi Lingshan Zhu,
>>>
>>> I have reviewed v4 version transport VQ a while back that consist of two main
>> pieces.
>>> 1. some virtio device creation/destroy commands 2. virtio device
>>> feature and config space provisioning commands.
>>>
>>> Many of us are seeing the value of device provisioning commands
>> with/without live migration.
>>> New virtio devices do not need to follow any existing of config space etc per
>> say for real hw.
>>> There is some value in reusing software but it may not be the prime driving
>> factor for brand new devices.
>>> This needs more discussion and not as_is done in the v4.
>>>
>>> So, if you are planning to revise your transport vq patches as admin
>> commands, please split the series into two.
>>> a. device and config space provisioning SET/GET part.
>> Hi Parav
>>
>> I agree it is good to unify the interface for SRIOV and SIOV. However more
>> discussion is required, e.g., for a VF, will config its MSIX by admin vq(owned by
>> host) conflict with the MSIX cap.
>>
> No, it doesn't conflict.
> SR-PCIM interface is outside the scope of PCIe spec.
> I completed the work in PCI spec to clarify this already, its coming in the 6.1 errata spec.
>
> Non virtio device and Linux kernel both already has similar implementation in place and in use for more than a year.
> So msix provisioning via AQ is fine. VF can expose new MSIX capability post reconfiguration via AQ.
It is the guest to config MSIX of an assigned VF by writing to MSIX 
capability. However host owns the AQ,
means host can modify the MSIX config even when guest operational running.
A new synchronization mechanism? Trap accesses to MSIX cap? Ban access 
of MSIX through AQ after DRIVER_OK?
Do you have any detailed information about how to prevent the conflicts 
like this?
>
> We will be able to unify them across SRIOV and SIOV.
> SIOV anyway has to wait as its undergoing significant changes currently.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-29  2:18     ` Zhu, Lingshan
@ 2023-06-29  2:23       ` Parav Pandit
  2023-06-29  2:38         ` Zhu, Lingshan
  0 siblings, 1 reply; 12+ messages in thread
From: Parav Pandit @ 2023-06-29  2:23 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: virtio-comment, Xuan Zhuo, virtio


> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> Sent: Wednesday, June 28, 2023 10:18 PM


> > So msix provisioning via AQ is fine. VF can expose new MSIX capability post
> reconfiguration via AQ.
> It is the guest to config MSIX of an assigned VF by writing to MSIX capability.
> However host owns the AQ, means host can modify the MSIX config even when
> guest operational running.

Host can do many things not just MSIX config.
Host is not supposed modify the config.

In some OS MSI-X actual values are not even written by the guest...

> A new synchronization mechanism? Trap accesses to MSIX cap? Ban access of
> MSIX through AQ after DRIVER_OK?
> Do you have any detailed information about how to prevent the conflicts like
> this?

A hypervisor is a trusted entity to not mess with the VF.
A hypervisor can go to an extreme to even do PCI FLR while guest is running..

So no need to go to the extreme.

When hypervisor is untrusted in relatively far future, when a VF is put in some secure enclave and locked hypervisor will not such access.
At that point large part will be covered.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-29  2:23       ` Parav Pandit
@ 2023-06-29  2:38         ` Zhu, Lingshan
  2023-06-29  2:47           ` Parav Pandit
  0 siblings, 1 reply; 12+ messages in thread
From: Zhu, Lingshan @ 2023-06-29  2:38 UTC (permalink / raw)
  To: Parav Pandit; +Cc: virtio-comment, Xuan Zhuo, virtio



On 6/29/2023 10:23 AM, Parav Pandit wrote:
>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
>> Sent: Wednesday, June 28, 2023 10:18 PM
>
>>> So msix provisioning via AQ is fine. VF can expose new MSIX capability post
>> reconfiguration via AQ.
>> It is the guest to config MSIX of an assigned VF by writing to MSIX capability.
>> However host owns the AQ, means host can modify the MSIX config even when
>> guest operational running.
> Host can do many things not just MSIX config.
> Host is not supposed modify the config.
>
> In some OS MSI-X actual values are not even written by the guest...
yest, current host stack is not perfect.
So lets resolve these issues first rather than introduce new attack surface.
>
>> A new synchronization mechanism? Trap accesses to MSIX cap? Ban access of
>> MSIX through AQ after DRIVER_OK?
>> Do you have any detailed information about how to prevent the conflicts like
>> this?
> A hypervisor is a trusted entity to not mess with the VF.
> A hypervisor can go to an extreme to even do PCI FLR while guest is running..
>
> So no need to go to the extreme.
>
> When hypervisor is untrusted in relatively far future, when a VF is put in some secure enclave and locked hypervisor will not such access.
> At that point large part will be covered.
There can be other malicious software and a hypervisor may compromise. I 
still think we should resolve the issues first.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-29  2:38         ` Zhu, Lingshan
@ 2023-06-29  2:47           ` Parav Pandit
  2023-06-29  4:01             ` Zhu, Lingshan
  0 siblings, 1 reply; 12+ messages in thread
From: Parav Pandit @ 2023-06-29  2:47 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: virtio-comment, Xuan Zhuo, virtio


> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> Sent: Wednesday, June 28, 2023 10:39 PM
> 
> 
> On 6/29/2023 10:23 AM, Parav Pandit wrote:
> >> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> >> Sent: Wednesday, June 28, 2023 10:18 PM
> >
> >>> So msix provisioning via AQ is fine. VF can expose new MSIX
> >>> capability post
> >> reconfiguration via AQ.
> >> It is the guest to config MSIX of an assigned VF by writing to MSIX capability.
> >> However host owns the AQ, means host can modify the MSIX config even
> >> when guest operational running.
> > Host can do many things not just MSIX config.
> > Host is not supposed modify the config.
> >
> > In some OS MSI-X actual values are not even written by the guest...
> yest, current host stack is not perfect.
> So lets resolve these issues first rather than introduce new attack surface.
> >
> >> A new synchronization mechanism? Trap accesses to MSIX cap? Ban
> >> access of MSIX through AQ after DRIVER_OK?
> >> Do you have any detailed information about how to prevent the
> >> conflicts like this?
> > A hypervisor is a trusted entity to not mess with the VF.
> > A hypervisor can go to an extreme to even do PCI FLR while guest is running..
> >
> > So no need to go to the extreme.
> >
> > When hypervisor is untrusted in relatively far future, when a VF is put in some
> secure enclave and locked hypervisor will not such access.
> > At that point large part will be covered.
> There can be other malicious software and a hypervisor may compromise. 

In any case it is not the role of the AQ etc to provide such protection etc.

Safeguarding from hypervisor is completely different topic using confidential computing, TDISP and other affiliated standards.
It is far beyond msix config piece to tackle.

Before we reach there, virtio spec has certain fundamental catch up to be done, at least to me.

> I still think we should resolve the issues first.
I don’t think this is really an issue.
Passthrough VF devices are already there for more than a decade and msix config is just one small piece of it to provision, among others.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-29  2:47           ` Parav Pandit
@ 2023-06-29  4:01             ` Zhu, Lingshan
  2023-06-29  4:11               ` Parav Pandit
  0 siblings, 1 reply; 12+ messages in thread
From: Zhu, Lingshan @ 2023-06-29  4:01 UTC (permalink / raw)
  To: Parav Pandit; +Cc: virtio-comment, Xuan Zhuo, virtio



On 6/29/2023 10:47 AM, Parav Pandit wrote:
>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
>> Sent: Wednesday, June 28, 2023 10:39 PM
>>
>>
>> On 6/29/2023 10:23 AM, Parav Pandit wrote:
>>>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
>>>> Sent: Wednesday, June 28, 2023 10:18 PM
>>>>> So msix provisioning via AQ is fine. VF can expose new MSIX
>>>>> capability post
>>>> reconfiguration via AQ.
>>>> It is the guest to config MSIX of an assigned VF by writing to MSIX capability.
>>>> However host owns the AQ, means host can modify the MSIX config even
>>>> when guest operational running.
>>> Host can do many things not just MSIX config.
>>> Host is not supposed modify the config.
>>>
>>> In some OS MSI-X actual values are not even written by the guest...
>> yest, current host stack is not perfect.
>> So lets resolve these issues first rather than introduce new attack surface.
>>>> A new synchronization mechanism? Trap accesses to MSIX cap? Ban
>>>> access of MSIX through AQ after DRIVER_OK?
>>>> Do you have any detailed information about how to prevent the
>>>> conflicts like this?
>>> A hypervisor is a trusted entity to not mess with the VF.
>>> A hypervisor can go to an extreme to even do PCI FLR while guest is running..
>>>
>>> So no need to go to the extreme.
>>>
>>> When hypervisor is untrusted in relatively far future, when a VF is put in some
>> secure enclave and locked hypervisor will not such access.
>>> At that point large part will be covered.
>> There can be other malicious software and a hypervisor may compromise.
> In any case it is not the role of the AQ etc to provide such protection etc.
Yes, it is not AQ's responsibility. I wish we don't introduce more 
weakness.
>
> Safeguarding from hypervisor is completely different topic using confidential computing, TDISP and other affiliated standards.
> It is far beyond msix config piece to tackle.
>
> Before we reach there, virtio spec has certain fundamental catch up to be done, at least to me.
So lets work on that first than rush into the AQ/VF provisioning case.
>
>> I still think we should resolve the issues first.
> I don’t think this is really an issue.
> Passthrough VF devices are already there for more than a decade and msix config is just one small piece of it to provision, among others.
passthrough, like vfio which assign the device including its config 
space to a guest, works well. While AQ is a side channel.


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-29  4:01             ` Zhu, Lingshan
@ 2023-06-29  4:11               ` Parav Pandit
  2023-06-29  6:35                 ` Zhu, Lingshan
  0 siblings, 1 reply; 12+ messages in thread
From: Parav Pandit @ 2023-06-29  4:11 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: virtio-comment, Xuan Zhuo, virtio


> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> Sent: Thursday, June 29, 2023 12:02 AM
> 
> On 6/29/2023 10:47 AM, Parav Pandit wrote:
> >> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> >> Sent: Wednesday, June 28, 2023 10:39 PM
> >>
> >>
> >> On 6/29/2023 10:23 AM, Parav Pandit wrote:
> >>>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> >>>> Sent: Wednesday, June 28, 2023 10:18 PM
> >>>>> So msix provisioning via AQ is fine. VF can expose new MSIX
> >>>>> capability post
> >>>> reconfiguration via AQ.
> >>>> It is the guest to config MSIX of an assigned VF by writing to MSIX
> capability.
> >>>> However host owns the AQ, means host can modify the MSIX config
> >>>> even when guest operational running.
> >>> Host can do many things not just MSIX config.
> >>> Host is not supposed modify the config.
> >>>
> >>> In some OS MSI-X actual values are not even written by the guest...
> >> yest, current host stack is not perfect.
> >> So lets resolve these issues first rather than introduce new attack surface.
> >>>> A new synchronization mechanism? Trap accesses to MSIX cap? Ban
> >>>> access of MSIX through AQ after DRIVER_OK?
> >>>> Do you have any detailed information about how to prevent the
> >>>> conflicts like this?
> >>> A hypervisor is a trusted entity to not mess with the VF.
> >>> A hypervisor can go to an extreme to even do PCI FLR while guest is
> running..
> >>>
> >>> So no need to go to the extreme.
> >>>
> >>> When hypervisor is untrusted in relatively far future, when a VF is
> >>> put in some
> >> secure enclave and locked hypervisor will not such access.
> >>> At that point large part will be covered.
> >> There can be other malicious software and a hypervisor may compromise.
> > In any case it is not the role of the AQ etc to provide such protection etc.
> Yes, it is not AQ's responsibility. I wish we don't introduce more weakness.
> >
> > Safeguarding from hypervisor is completely different topic using confidential
> computing, TDISP and other affiliated standards.
> > It is far beyond msix config piece to tackle.
> >
> > Before we reach there, virtio spec has certain fundamental catch up to be
> done, at least to me.
> So lets work on that first than rush into the AQ/VF provisioning case.
> >
I mean querying and provisioning feature bits and config space fields is the basic functionality before attempting to securing it.
I was told that you are going to rebase the tvq series on top of AQ, hence the ask to split the series to two.
If I misunderstood, its fine, my bad.
Someone else will pick up the features/config provisioning+query part.

> >> I still think we should resolve the issues first.
> > I don’t think this is really an issue.
> > Passthrough VF devices are already there for more than a decade and msix
> config is just one small piece of it to provision, among others.
> passthrough, like vfio which assign the device including its config space to a
> guest, works well. While AQ is a side channel.
Sure. AQ is side channel but hypervisor's ability to access VF while in use including doing FLR and more is already exist.
So such thing does not need special spec level protection.
An OS driver can handle it internally.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-29  4:11               ` Parav Pandit
@ 2023-06-29  6:35                 ` Zhu, Lingshan
  2023-06-29 16:47                   ` Parav Pandit
  0 siblings, 1 reply; 12+ messages in thread
From: Zhu, Lingshan @ 2023-06-29  6:35 UTC (permalink / raw)
  To: Parav Pandit; +Cc: virtio-comment, Xuan Zhuo, virtio



On 6/29/2023 12:11 PM, Parav Pandit wrote:
>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
>> Sent: Thursday, June 29, 2023 12:02 AM
>>
>> On 6/29/2023 10:47 AM, Parav Pandit wrote:
>>>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
>>>> Sent: Wednesday, June 28, 2023 10:39 PM
>>>>
>>>>
>>>> On 6/29/2023 10:23 AM, Parav Pandit wrote:
>>>>>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
>>>>>> Sent: Wednesday, June 28, 2023 10:18 PM
>>>>>>> So msix provisioning via AQ is fine. VF can expose new MSIX
>>>>>>> capability post
>>>>>> reconfiguration via AQ.
>>>>>> It is the guest to config MSIX of an assigned VF by writing to MSIX
>> capability.
>>>>>> However host owns the AQ, means host can modify the MSIX config
>>>>>> even when guest operational running.
>>>>> Host can do many things not just MSIX config.
>>>>> Host is not supposed modify the config.
>>>>>
>>>>> In some OS MSI-X actual values are not even written by the guest...
>>>> yest, current host stack is not perfect.
>>>> So lets resolve these issues first rather than introduce new attack surface.
>>>>>> A new synchronization mechanism? Trap accesses to MSIX cap? Ban
>>>>>> access of MSIX through AQ after DRIVER_OK?
>>>>>> Do you have any detailed information about how to prevent the
>>>>>> conflicts like this?
>>>>> A hypervisor is a trusted entity to not mess with the VF.
>>>>> A hypervisor can go to an extreme to even do PCI FLR while guest is
>> running..
>>>>> So no need to go to the extreme.
>>>>>
>>>>> When hypervisor is untrusted in relatively far future, when a VF is
>>>>> put in some
>>>> secure enclave and locked hypervisor will not such access.
>>>>> At that point large part will be covered.
>>>> There can be other malicious software and a hypervisor may compromise.
>>> In any case it is not the role of the AQ etc to provide such protection etc.
>> Yes, it is not AQ's responsibility. I wish we don't introduce more weakness.
>>> Safeguarding from hypervisor is completely different topic using confidential
>> computing, TDISP and other affiliated standards.
>>> It is far beyond msix config piece to tackle.
>>>
>>> Before we reach there, virtio spec has certain fundamental catch up to be
>> done, at least to me.
>> So lets work on that first than rush into the AQ/VF provisioning case.
> I mean querying and provisioning feature bits and config space fields is the basic functionality before attempting to securing it.
It may be fine to _ONLY_ provision virtio configuration through AQ, 
limit the changes in virtio spec.
Actually this is not provisioning a VF, this is modification after 
creation, because we still
create a VF through PCI interface num_vfs.
> I was told that you are going to rebase the tvq series on top of AQ, hence the ask to split the series to two.
> If I misunderstood, its fine, my bad.
> Someone else will pick up the features/config provisioning+query part.
If you want to re-use some transport cmds for SRIOV, that's fine, we 
just can not reuse the whole command set for SRIOV.
For example, we expect AQ as a transport for SIOV, therefore MSIX is 
config-ed through AQ, as we discussed this
set_msix_cmd may not apply to SRIOV for now.

Querying is fine as you know it is read only.
For setting, I assume we need a new mechanism to sycn the PCI config 
space with AQ, and still the host owns the AQ.

>
>>>> I still think we should resolve the issues first.
>>> I don’t think this is really an issue.
>>> Passthrough VF devices are already there for more than a decade and msix
>> config is just one small piece of it to provision, among others.
>> passthrough, like vfio which assign the device including its config space to a
>> guest, works well. While AQ is a side channel.
> Sure. AQ is side channel but hypervisor's ability to access VF while in use including doing FLR and more is already exist.
a hypervisor should not reset anything when the device has been 
passthroughed, or such a hypervisor may be considered buggy
> So such thing does not need special spec level protection.
> An OS driver can handle it internally.
again, I wish we don't introduce more attacking surface even the stack 
is already vulnerable.

Thanks


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 12+ messages in thread

* RE: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-29  6:35                 ` Zhu, Lingshan
@ 2023-06-29 16:47                   ` Parav Pandit
  2023-06-30  3:09                     ` Zhu, Lingshan
  0 siblings, 1 reply; 12+ messages in thread
From: Parav Pandit @ 2023-06-29 16:47 UTC (permalink / raw)
  To: Zhu, Lingshan; +Cc: virtio-comment, Xuan Zhuo, virtio


> From: Zhu, Lingshan <lingshan.zhu@intel.com>
> Sent: Thursday, June 29, 2023 2:36 AM

> > I mean querying and provisioning feature bits and config space fields is the
> basic functionality before attempting to securing it.
> It may be fine to _ONLY_ provision virtio configuration through AQ, limit the
> changes in virtio spec.
> Actually this is not provisioning a VF, this is modification after creation, because
> we still create a VF through PCI interface num_vfs.

It is a modification before a VF is given to the guest.

> > I was told that you are going to rebase the tvq series on top of AQ, hence the
> ask to split the series to two.
> > If I misunderstood, its fine, my bad.
> > Someone else will pick up the features/config provisioning+query part.
> If you want to re-use some transport cmds for SRIOV, that's fine, we just can not
> reuse the whole command set for SRIOV.
Sure.

> For example, we expect AQ as a transport for SIOV, therefore MSIX is config-ed
> through AQ, as we discussed this set_msix_cmd may not apply to SRIOV for
> now.
>
MSIX provisioning for SIOV and SRIOV applies through the AQ.
Configuring the MSIX entry itself by the device is not through the AQ, it must a direct channel without mediation.
This is different discussion to do when SIOV R2 comes in place, the spec is under WIP so no point in debating for now.
 
> Querying is fine as you know it is read only.
> For setting, I assume we need a new mechanism to sycn the PCI config space
> with AQ, and still the host owns the AQ.
> 
There is no good reason establish for this need. So I don’t the point of only asserting. :)
Hypervisor sw already takes care to not mess and VFs given to the VM and does not touch it.
Hence, I don’t see a need for over engineering here.
Once there is solid race condition explained that requires sync, it is better to do that time.

> >
> >>>> I still think we should resolve the issues first.
> >>> I don’t think this is really an issue.
> >>> Passthrough VF devices are already there for more than a decade and
> >>> msix
> >> config is just one small piece of it to provision, among others.
> >> passthrough, like vfio which assign the device including its config
> >> space to a guest, works well. While AQ is a side channel.
> > Sure. AQ is side channel but hypervisor's ability to access VF while in use
> including doing FLR and more is already exist.
> a hypervisor should not reset anything when the device has been
> passthroughed, or such a hypervisor may be considered buggy
> > So such thing does not need special spec level protection.
> > An OS driver can handle it internally.
> again, I wish we don't introduce more attacking surface even the stack is already
> vulnerable.
There is no attack surface added. It is withing the hypervisor boundary owned by the owner device driver.
How to access a owner driver is role of hypervisor OS like how its done today for virtio and non virtio devices, not really the virtio spec.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [virtio-comment] Re: virtio member device provisioning get/set and virtio child device life cycle mixing not needed
  2023-06-29 16:47                   ` Parav Pandit
@ 2023-06-30  3:09                     ` Zhu, Lingshan
  0 siblings, 0 replies; 12+ messages in thread
From: Zhu, Lingshan @ 2023-06-30  3:09 UTC (permalink / raw)
  To: Parav Pandit; +Cc: virtio-comment, Xuan Zhuo, virtio



On 6/30/2023 12:47 AM, Parav Pandit wrote:
>> From: Zhu, Lingshan <lingshan.zhu@intel.com>
>> Sent: Thursday, June 29, 2023 2:36 AM
>>> I mean querying and provisioning feature bits and config space fields is the
>> basic functionality before attempting to securing it.
>> It may be fine to _ONLY_ provision virtio configuration through AQ, limit the
>> changes in virtio spec.
>> Actually this is not provisioning a VF, this is modification after creation, because
>> we still create a VF through PCI interface num_vfs.
> It is a modification before a VF is given to the guest.
IMHO we should prevent modification through AQ after assigning a VF the 
the guest
>
>>> I was told that you are going to rebase the tvq series on top of AQ, hence the
>> ask to split the series to two.
>>> If I misunderstood, its fine, my bad.
>>> Someone else will pick up the features/config provisioning+query part.
>> If you want to re-use some transport cmds for SRIOV, that's fine, we just can not
>> reuse the whole command set for SRIOV.
> Sure.
>
>> For example, we expect AQ as a transport for SIOV, therefore MSIX is config-ed
>> through AQ, as we discussed this set_msix_cmd may not apply to SRIOV for
>> now.
>>
> MSIX provisioning for SIOV and SRIOV applies through the AQ.
> Configuring the MSIX entry itself by the device is not through the AQ, it must a direct channel without mediation.
Not sure, without AQ, current live migration solution still migrates 
MSIX through the MSIX cap config space before
assigning the device to a guest.

And still, need to prevent modification through AQ after passthrough the 
device to a guest
> This is different discussion to do when SIOV R2 comes in place, the spec is under WIP so no point in debating for now.
so TRUE, however the core concept of SIOV is constant in R2 till now.

Let's discuss the details and decide how to prevent potential issues and 
how to reuse the cmds in my rebasing series.
>   
>> Querying is fine as you know it is read only.
>> For setting, I assume we need a new mechanism to sycn the PCI config space
>> with AQ, and still the host owns the AQ.
>>
> There is no good reason establish for this need. So I don’t the point of only asserting. :)
> Hypervisor sw already takes care to not mess and VFs given to the VM and does not touch it.
> Hence, I don’t see a need for over engineering here.
> Once there is solid race condition explained that requires sync, it is better to do that time.
There are some buggy hypervisors, don't trust them. Lets discuss the 
details in my rebasing series.
>
>>>>>> I still think we should resolve the issues first.
>>>>> I don’t think this is really an issue.
>>>>> Passthrough VF devices are already there for more than a decade and
>>>>> msix
>>>> config is just one small piece of it to provision, among others.
>>>> passthrough, like vfio which assign the device including its config
>>>> space to a guest, works well. While AQ is a side channel.
>>> Sure. AQ is side channel but hypervisor's ability to access VF while in use
>> including doing FLR and more is already exist.
>> a hypervisor should not reset anything when the device has been
>> passthroughed, or such a hypervisor may be considered buggy
>>> So such thing does not need special spec level protection.
>>> An OS driver can handle it internally.
>> again, I wish we don't introduce more attacking surface even the stack is already
>> vulnerable.
> There is no attack surface added. It is withing the hypervisor boundary owned by the owner device driver.
> How to access a owner driver is role of hypervisor OS like how its done today for virtio and non virtio devices, not really the virtio spec.
There is still a slight different, for SRIOV passthrough, vfio or vDPA 
owns the device as well as its config space, but AQ is a side channel as
we discussed before, so we must pick the command set that can be reused 
in SRIOV carefully. I will try

Thanks


This publicly archived list offers a means to provide input to the
OASIS Virtual I/O Device (VIRTIO) TC.

In order to verify user consent to the Feedback License terms and
to minimize spam in the list archive, subscription is required
before posting.

Subscribe: virtio-comment-subscribe@lists.oasis-open.org
Unsubscribe: virtio-comment-unsubscribe@lists.oasis-open.org
List help: virtio-comment-help@lists.oasis-open.org
List archive: https://lists.oasis-open.org/archives/virtio-comment/
Feedback License: https://www.oasis-open.org/who/ipr/feedback_license.pdf
List Guidelines: https://www.oasis-open.org/policies-guidelines/mailing-lists
Committee: https://www.oasis-open.org/committees/virtio/
Join OASIS: https://www.oasis-open.org/join/


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-06-30  3:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-28  0:22 [virtio-comment] virtio member device provisioning get/set and virtio child device life cycle mixing not needed Parav Pandit
2023-06-28  7:30 ` [virtio-comment] " Zhu, Lingshan
2023-06-28 13:10   ` Parav Pandit
2023-06-29  2:18     ` Zhu, Lingshan
2023-06-29  2:23       ` Parav Pandit
2023-06-29  2:38         ` Zhu, Lingshan
2023-06-29  2:47           ` Parav Pandit
2023-06-29  4:01             ` Zhu, Lingshan
2023-06-29  4:11               ` Parav Pandit
2023-06-29  6:35                 ` Zhu, Lingshan
2023-06-29 16:47                   ` Parav Pandit
2023-06-30  3:09                     ` Zhu, Lingshan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.