Re: [PATCH] Re-enable LTR/OBFF when device is owned by pciback

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Sun, Apr 22, 2012 at 03:25:04PM +0000, Hao, Xudong wrote:
> When PCIE device which has LTR/OBFF capabilities is owned by pciback, LTR/OBFF feature may be disabled. This patch re-enable LTR and OBFF, so that guest with device assigned can be benefitted.

Shouldn't they be reset when the device is de-assigned from the guest as well?
> 
> Signed-off-by: Xudong Hao <xudong.hao@intel.com>
> 
> diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
> index 19834d1..82aac5b 100644
> --- a/drivers/xen/xen-pciback/pci_stub.c
> +++ b/drivers/xen/xen-pciback/pci_stub.c
> @@ -334,6 +334,14 @@ static int __devinit pcistub_init_device(struct pci_dev *dev)
>  	dev_dbg(&dev->dev, "reset device\n");
>  	xen_pcibk_reset_device(dev);
>  
> +	/* set default value */
> +	unsigned long type = PCI_EXP_OBFF_SIGNAL_ALWAYS;
> +	int snoop_lat_ns = 1024, nosnoop_lat_ns = 1024; /* 1024ns is max value */


Please put them in the appropiate location - at the start of the 
function.

> +
> +	/* Enable LTR and OBFF before do device assignment */
> +	/* LTR(Latency tolerance reporting) allows devices to send messages to the
> +	 * root complex indicating their latency tolerance for snooped & unsnooped
> +	 * memory transactions. 
> +	 */
> +	pci_enable_ltr(dev);
> +	pci_set_ltr(dev, snoop_lat_ns, nosnoop_lat_ns);

OK, but didn't you just come with up those values?

> +
> +	/* OBFF (optimized buffer flush/fill), where supported, can help improve
> +	 * energy efficiency by giving devices information about when interrupts and
> +	 * other activity will have a reduced power impact.
> +	 */
> +	pci_enable_obff(dev, type);
> +
>  	dev->dev_flags |= PCI_DEV_FLAGS_ASSIGNED;
>  	return 0;