[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [virtio-dev] Re: [EXT] Re: [virtio-dev] Re: [RFC PATCH] virtio-iommu: Add PAGE_SIZE_MASK property
Hi Jean, Bharat, On 3/27/20 10:35 AM, Jean-Philippe Brucker wrote: > On Fri, Mar 27, 2020 at 09:17:43AM +0000, Bharat Bhushan wrote: >> >> Sent again, somehow the email-address got corrupted. >> >>> -----Original Message----- >>> From: Bharat Bhushan >>> Sent: Friday, March 27, 2020 2:46 PM >>> To: @mx0a-0016f401.pphosted.com >>> Cc: Auger Eric <eric.auger@redhat.com>; virtio-dev@lists.oasis-open.org >>> Subject: RE: [EXT] Re: [virtio-dev] Re: [RFC PATCH] virtio-iommu: Add >>> PAGE_SIZE_MASK property >>> >>> Hi Jean, >>> >>>> -----Original Message----- >>>> From: Jean-Philippe Brucker <jean-philippe@linaro.org> >>>> Sent: Friday, March 27, 2020 2:31 PM >>>> To: Bharat Bhushan <bbhushan2@marvell.com> >>>> Cc: Auger Eric <eric.auger@redhat.com>; >>>> virtio-dev@lists.oasis-open.org >>>> Subject: Re: [EXT] Re: [virtio-dev] Re: [RFC PATCH] virtio-iommu: Add >>>> PAGE_SIZE_MASK property >>>> >>>> On Fri, Mar 27, 2020 at 05:20:56AM +0000, Bharat Bhushan wrote: >>>>> Hi Jean, >>>>> >>>>>> -----Original Message----- >>>>>> From: Auger Eric <eric.auger@redhat.com> >>>>>> Sent: Thursday, March 26, 2020 4:50 PM >>>>>> To: Jean-Philippe Brucker <jean-philippe@linaro.org> >>>>>> Cc: virtio-dev@lists.oasis-open.org; Bharat Bhushan >>>>>> <bbhushan2@marvell.com> >>>>>> Subject: [EXT] Re: [virtio-dev] Re: [RFC PATCH] virtio-iommu: Add >>>>>> PAGE_SIZE_MASK property >>>>>> >>>>>> External Email >>>>>> >>>>>> ------------------------------------------------------------------ >>>>>> -- >>>>>> -- >>>>>> Hi Jean, >>>>>> >>>>>> On 3/26/20 11:49 AM, Jean-Philippe Brucker wrote: >>>>>>> On Mon, Mar 23, 2020 at 03:22:40PM +0100, Auger Eric wrote: >>>>>>>> Hi Jean, >>>>>>>> >>>>>>>> On 3/23/20 2:38 PM, Jean-Philippe Brucker wrote: >>>>>>>>> Add a PROBE property to declare the mapping granularity per endpoint. >>>>>>>>> The virtio-iommu device already declares a granule in its >>>>>>>>> config space, but when endpoints are behind different physical >>>>>>>>> IOMMUs, they may have different mapping granules. This new >>>>>>>>> property allows to override the global page_size_mask for each endpoint. >>>>>>>>> >>>>>>>>> In the future it may be useful to describe more than one >>>>>>>>> page_size_mask for each endpoint, and allow them to negotiate >>>>>>>>> it during ATTACH. For example two masks could allow the driver >>>>>>>>> to choose between 4k and 64k granule, along with their >>>>>>>>> respective block mapping sizes. This could be added by >>>>>>>>> replacing \field{reserved} with an array >>>>>> length, for example. >>>>>>>> Sorry I don't get the use case where several page size bitmaps >>>>>>>> should be exposed. >>>>>>> >>>>>>> For a 4k granule you get block mappings of 2M and 1G. For a 64k >>>>>>> granule you get 512M and 4T block mappings. If you want to >>>>>>> communicate both options to the guest, you need two separate >>>>>>> masks, 0x40201000 and 0x40020010000. Then the guest could choose >>>>>>> one of the granules during attach, if we add a flag to the >>>>>>> attach request. I'm not suggesting we do that now, just trying >>>>>>> to make sure it can be extended if anyone actually wants it. >>>>>>> Personally I don't think it's worth adding, especially given the >>>>>>> additional work required in >>>> the host. >>>>>> OK I get it now. >>>>> >>>>> What some clarification about two page-size-mask configurations available. >>>>> - Global configuration for page-size-mask >>>>> - per endpoint page-size-mask configuration >>>>> >>>>> PAGE_SIZE_MASK probe for and endpoint can return zero or non-zero value. >>>>> If it returns non-zero value than it will override the global configuration. >>>>> If PAGE_SIZE_MASK probe for and endpoint return zero value than >>>>> global page- >>>> size-mask configuration will be used. >>>>> >>>>> Is that correct? >>>> >>>> Yes. If a PAGE_SIZE_MASK property is available for an endpoint, the >>>> driver should use that mask. Otherwise it should use the global mask, which is >>> always provided. >>> >>> That mean even if the device return ZERO as page-size mask it will override global >>> page-size-mask configuration? >>> So device should not return PAGE_SIZE_MASK property when page-size-mask not >>> set for that endpoint, that is zero? >>> >>> Or >>> >>> Device can return page-size-mask = 0 for the endpoint in PAGE_SIZE_MASK >>> property, driver have to use global page-size-mask when PAGE_SIZE_MASK >>> property returns page-size-mask = 0. >>> > > Ah sorry I didn't get the question. page_size_mask == 0 isn't valid: > > The device MUST set at least one bit in \field{page_size_mask}, > describing the page granularity. The device MAY set more than one > bit in \field{page_size_mask}. > > So the driver doesn't have to expect a value of 0, that would be a device > bug. If it wants to be on the safe side, then it rejects a page_size_mask > of 0 and use the global mask instead. So I think on device's side, the best is to not return any PAGE_SIZE_MASK property for an ep which does not have any special limitation (for instance a virtio EP), as opposed to a VFIO EP, right? Supporting this probe property does not mean we always report it, right? > >>>> I wonder, should we introduce some form of negotiation now? If the >>>> driver doesn't know about the new probe property, it will use the >>>> global mask. At some point it will send a MAP request not aligned on >>>> the page granule, and the device will abort the request. If instead we >>>> add a flag and page mask field to the attach request, the device would >>>> know that the driver didn't understand the per-endpoint page mask and abort >>> the attach. >>> >>> What if device support PAGE_SIZE_MASK property then it will always return the >>> property, with zero or non-zero mask. >>> If zero mask, use global mask otherwise use per-endpoint mask. > > My question is about device supports PAGE_SIZE_MASK property and driver > doesn't. For example a linux v4.6 guest would ignore the PAGE_SIZE_MASK > property. Then it will use the global mask, and send MAP requests that > aren't aligned on the per-endpoint page granule (they fail with S_RANGE > status). Should we, with the introduction of the PAGE_SIZE_MASK property, > also introduce a page size negotiation mechanism? So that the device > knows early whether the guest understands or not the provided > PAGE_SIZE_MASK property? On std iommu drivers, the check also is done on map, right? If so, and given the current known user pool, I would leave it as it is today. Thanks Eric > > Thanks, > Jean > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org >
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]