Re: [PATCH RESEND v4] nvme: introduce PMR support from NVMe 1.4 spec

2020-04-08 Thread Kevin Wolf
Am 30.03.2020 um 18:55 hat Keith Busch geschrieben:
> On Mon, Mar 30, 2020 at 09:46:56AM -0700, Andrzej Jakowski wrote:
> > This patch introduces support for PMR that has been defined as part of NVMe 
> > 1.4
> > spec. User can now specify a pmrdev option that should point to 
> > HostMemoryBackend.
> > pmrdev memory region will subsequently be exposed as PCI BAR 2 in emulated 
> > NVMe
> > device. Guest OS can perform mmio read and writes to the PMR region that 
> > will stay
> > persistent across system reboot.
> 
> Looks pretty good to me.
> 
> Reviewed-by: Keith Busch 

Thanks, applied to the block-next branch for 5.1.

Kevin




Re: [PATCH RESEND v4] nvme: introduce PMR support from NVMe 1.4 spec

2020-03-31 Thread Andrzej Jakowski
On 3/30/20 12:31 PM, Klaus Birkelund Jensen wrote:
> On Mar 31 03:13, Keith Busch wrote:
>> On Mon, Mar 30, 2020 at 08:07:32PM +0200, Klaus Birkelund Jensen wrote:
>>> On Mar 31 01:55, Keith Busch wrote:
 On Mon, Mar 30, 2020 at 09:46:56AM -0700, Andrzej Jakowski wrote:
> This patch introduces support for PMR that has been defined as part of 
> NVMe 1.4
> spec. User can now specify a pmrdev option that should point to 
> HostMemoryBackend.
> pmrdev memory region will subsequently be exposed as PCI BAR 2 in 
> emulated NVMe
> device. Guest OS can perform mmio read and writes to the PMR region that 
> will stay
> persistent across system reboot.

 Looks pretty good to me.

 Reviewed-by: Keith Busch 

 For a possible future extention, it could be interesting to select the
 BAR for PMR dynamically, so that you could have CMB and PMR enabled on
 the same device.

>>>
>>> I thought about the same thing, but this would require disabling MSI-X
>>> right? Otherwise there is not enough room. AFAIU, the iomem takes up
>>> BAR0 and BAR1, CMB (or PMR) uses BAR2 and BAR3 and MSI-X uses BAR4 or 5.
>>
>> That's the way the emulated device is currently set, but there's no
>> reason for CMB or MSIx to use an exlusive bar. Both of those can be
>> be offsets into BAR 0/1, for example. PMR is the only one that can't
>> share.
>>
> 
> Oh, I did not consider that at all. Thanks!
> 
Makes sense - I didn't realize that it is possible. Thx Keith! 
Let me propose to introduce it in a separate patch.



Re: [PATCH RESEND v4] nvme: introduce PMR support from NVMe 1.4 spec

2020-03-30 Thread Klaus Birkelund Jensen
On Mar 31 03:13, Keith Busch wrote:
> On Mon, Mar 30, 2020 at 08:07:32PM +0200, Klaus Birkelund Jensen wrote:
> > On Mar 31 01:55, Keith Busch wrote:
> > > On Mon, Mar 30, 2020 at 09:46:56AM -0700, Andrzej Jakowski wrote:
> > > > This patch introduces support for PMR that has been defined as part of 
> > > > NVMe 1.4
> > > > spec. User can now specify a pmrdev option that should point to 
> > > > HostMemoryBackend.
> > > > pmrdev memory region will subsequently be exposed as PCI BAR 2 in 
> > > > emulated NVMe
> > > > device. Guest OS can perform mmio read and writes to the PMR region 
> > > > that will stay
> > > > persistent across system reboot.
> > > 
> > > Looks pretty good to me.
> > > 
> > > Reviewed-by: Keith Busch 
> > > 
> > > For a possible future extention, it could be interesting to select the
> > > BAR for PMR dynamically, so that you could have CMB and PMR enabled on
> > > the same device.
> > > 
> > 
> > I thought about the same thing, but this would require disabling MSI-X
> > right? Otherwise there is not enough room. AFAIU, the iomem takes up
> > BAR0 and BAR1, CMB (or PMR) uses BAR2 and BAR3 and MSI-X uses BAR4 or 5.
> 
> That's the way the emulated device is currently set, but there's no
> reason for CMB or MSIx to use an exlusive bar. Both of those can be
> be offsets into BAR 0/1, for example. PMR is the only one that can't
> share.
> 

Oh, I did not consider that at all. Thanks!



Re: [PATCH RESEND v4] nvme: introduce PMR support from NVMe 1.4 spec

2020-03-30 Thread Keith Busch
On Mon, Mar 30, 2020 at 08:07:32PM +0200, Klaus Birkelund Jensen wrote:
> On Mar 31 01:55, Keith Busch wrote:
> > On Mon, Mar 30, 2020 at 09:46:56AM -0700, Andrzej Jakowski wrote:
> > > This patch introduces support for PMR that has been defined as part of 
> > > NVMe 1.4
> > > spec. User can now specify a pmrdev option that should point to 
> > > HostMemoryBackend.
> > > pmrdev memory region will subsequently be exposed as PCI BAR 2 in 
> > > emulated NVMe
> > > device. Guest OS can perform mmio read and writes to the PMR region that 
> > > will stay
> > > persistent across system reboot.
> > 
> > Looks pretty good to me.
> > 
> > Reviewed-by: Keith Busch 
> > 
> > For a possible future extention, it could be interesting to select the
> > BAR for PMR dynamically, so that you could have CMB and PMR enabled on
> > the same device.
> > 
> 
> I thought about the same thing, but this would require disabling MSI-X
> right? Otherwise there is not enough room. AFAIU, the iomem takes up
> BAR0 and BAR1, CMB (or PMR) uses BAR2 and BAR3 and MSI-X uses BAR4 or 5.

That's the way the emulated device is currently set, but there's no
reason for CMB or MSIx to use an exlusive bar. Both of those can be
be offsets into BAR 0/1, for example. PMR is the only one that can't
share.



Re: [PATCH RESEND v4] nvme: introduce PMR support from NVMe 1.4 spec

2020-03-30 Thread Klaus Birkelund Jensen
On Mar 31 01:55, Keith Busch wrote:
> On Mon, Mar 30, 2020 at 09:46:56AM -0700, Andrzej Jakowski wrote:
> > This patch introduces support for PMR that has been defined as part of NVMe 
> > 1.4
> > spec. User can now specify a pmrdev option that should point to 
> > HostMemoryBackend.
> > pmrdev memory region will subsequently be exposed as PCI BAR 2 in emulated 
> > NVMe
> > device. Guest OS can perform mmio read and writes to the PMR region that 
> > will stay
> > persistent across system reboot.
> 
> Looks pretty good to me.
> 
> Reviewed-by: Keith Busch 
> 
> For a possible future extention, it could be interesting to select the
> BAR for PMR dynamically, so that you could have CMB and PMR enabled on
> the same device.
> 

I thought about the same thing, but this would require disabling MSI-X
right? Otherwise there is not enough room. AFAIU, the iomem takes up
BAR0 and BAR1, CMB (or PMR) uses BAR2 and BAR3 and MSI-X uses BAR4 or 5.



Re: [PATCH RESEND v4] nvme: introduce PMR support from NVMe 1.4 spec

2020-03-30 Thread Keith Busch
On Mon, Mar 30, 2020 at 09:46:56AM -0700, Andrzej Jakowski wrote:
> This patch introduces support for PMR that has been defined as part of NVMe 
> 1.4
> spec. User can now specify a pmrdev option that should point to 
> HostMemoryBackend.
> pmrdev memory region will subsequently be exposed as PCI BAR 2 in emulated 
> NVMe
> device. Guest OS can perform mmio read and writes to the PMR region that will 
> stay
> persistent across system reboot.

Looks pretty good to me.

Reviewed-by: Keith Busch 

For a possible future extention, it could be interesting to select the
BAR for PMR dynamically, so that you could have CMB and PMR enabled on
the same device.