Xen devel - Jul 2008 - [PATCH] Improve the current FLR logic

Hi, all,
The attached patches try to improve the current FLR logic.
The idea is: removing the FLR logic from hypervisor and adding the
improved logic in Control Panel.

The current FLR logic in hypervisor has some issues: 1) Dstate
transition is not guaranteed to properly clear the device state; 2) the
current code for PCIe FLR is actually buggy: PCI_EXP_DEVSTA_TRPND
doesn''t mean the completion of FLR; according to the PCIe spec, after
issuing FLR, we should wait at least 100ms.

To make it easier to improve the FLR logic, and to make the hypervisor
thin, I think we might as well move the logic to Control Panel for the
time being. In the long run, the essential logic may be implemented in
the pciback driver of Dom0 instead.

[PATCH1] sysmnt_cleanup.patch: this is only a small cleanup.

[PATCH2] remove_FLR_in_xen.patch: remove the FLR logic in Xen.

[PATCH3] do_FLR_in_control_panel.patch:  the improved FLR logic in
Control Panel: 1) If the device is PCIe endpoint and supports PCIe FLR,
we use that; 2) Else, if the device is PCIe endpoint, and all functions
on the device are assigned to the same guest, we use the immediate
parent bus''s Secondary Bus Reset to reset all functions of the device
(here, actually we require all the functions of the device be assigned
to the same guest); 3) Else, if the device is PCI endpoint and is on a
host bus (e.g. integrated devices) and if the device supports PCI
Advanced Capabilities, we use that for FLR; 4) Else, we use the
Secondary Bus Reset (if the PCI device is behind a PCI/PCI-X bridge,
then all devices behind the uppermost such PCI/PCI-X bridge above this
device must be co-assigned)

[PATCH4] list_assignable_devices.patch: a command "xm
pci-list-assignable-devices" which can list all the assignable devices
in the system. It is useful for end users.

I made some tests on my hosts. Looks the patches work well.

I''d like to ask for your comments, and test feedbacks. Thank you very
much!

 Thanks, 
-- Dexuan





_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Maybe I''ve got this wrong, but it looks like you''re doing the
FLR
after the device has been deassigned (i.e., given back to dom0).
Shouldn''t you do the FLR before you actually deassign the device
instead?  If the device is currently set up for doing DMA transactions
to host memory, the pending transactions will end up in dom0''s memory
space.  This is fine if GMFN == MFN since the guest memory has still
not been released, but for GMFN != MFN you''ll end up corrupting
arbitrary dom0 memory.

The right procedure for deassigning devices should be:

  - Revoke device resources.
  - FLR.
  - Reassign device to dom0.

Likewise, for assigning devices we should do:

  - FLR.
  - Assign device to guest.
  - Grant device resources to guest.

Another option is to completely deassign the devices from the IOMMU
rather than reassigning them to dom0.  Devices bound to pciback need
not be assigned to dom0''s IOMMU tables anyway.

Further, your patch probably breaks things when running old dom0/xend
on a new hypervisor since you''ll end up not doing any FLR at all.  I
recently experienced the same thing with some of my own PCI cleanup
patches.

	eSk


[Dexuan Cui]
> Hi, all,
> The attached patches try to improve the current FLR logic.
> The idea is: removing the FLR logic from hypervisor and adding the
> improved logic in Control Panel.
> The current FLR logic in hypervisor has some issues: 1) Dstate
> transition is not guaranteed to properly clear the device state; 2) the
> current code for PCIe FLR is actually buggy: PCI_EXP_DEVSTA_TRPND
> doesn''t mean the completion of FLR; according to the PCIe spec,
after
> issuing FLR, we should wait at least 100ms.
> To make it easier to improve the FLR logic, and to make the hypervisor
> thin, I think we might as well move the logic to Control Panel for the
> time being. In the long run, the essential logic may be implemented in
> the pciback driver of Dom0 instead.
> [...]
> I made some tests on my hosts. Looks the patches work well.
> I''d like to ask for your comments, and test feedbacks. Thank you
> very much!
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi,

Cui, Dexuan wrote:
> [PATCH3] do_FLR_in_control_panel.patch:  the improved FLR logic in
> Control Panel: 1) If the device is PCIe endpoint and supports PCIe FLR,
> we use that; 2) Else, if the device is PCIe endpoint, and all functions
> on the device are assigned to the same guest, we use the immediate
> parent bus''s Secondary Bus Reset to reset all functions of the
device
> (here, actually we require all the functions of the device be assigned
> to the same guest); 3) Else, if the device is PCI endpoint and is on a
> host bus (e.g. integrated devices) and if the device supports PCI
> Advanced Capabilities, we use that for FLR; 4) Else, we use the
> Secondary Bus Reset (if the PCI device is behind a PCI/PCI-X bridge,
> then all devices behind the uppermost such PCI/PCI-X bridge above this
> device must be co-assigned)
In case 2) or 4), I think multfunction devices or sibling devices need
not be actually assigned to the same guest, if they are just owned by
pciback and not used by other guests.
> [PATCH4] list_assignable_devices.patch: a command "xm
> pci-list-assignable-devices" which can list all the assignable devices
> in the system. It is useful for end users.
It looks like xm_pci_list_assignable_devices() directly accesses to
sysfs and invokes hypercall without going through xend. Since xm
commands can be executed by a remote server through xmlrpc, most of
this function should be implemented in xend.
Also please note that test_assign_device() excludes the case of
non-iommu device assignment for pv.

Thanks,
-- Yosuke Iwamatsu

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Sat, 12 Jul 2008 20:37:34 +0800
"Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> I''d like to ask for your comments, and test feedbacks. Thank you
very
> much!
There is a problem with the device reconfiguring logic. xend saves all
Configuration Register''s values before reset. And xend writes the
values to the registers after reset. This means the resister''s values
which are configured by guest software are restore. Guest software
setting should be cleared.

I think the following Configuration Resister should be reconfigured to
correct values after reset. And other registers should not be
reconfigured after reset if there is no reason.

- It is necessary to write the base address of the resource allocated
  by the dom0 kernel to the following resister.
    - Base Address Register

- When dom0 starts, the values configured by firmware should be saved,
  and the values should be written to the following resisters after
  reset. The reason is that firmware configures them to archive system
  specific functions. Especially, the registers relating error reporting
  are configured to collect error information. If this configured values
  is changed, some functions might be lost.
  Instead of save/restore, it is good way to write the value from _HPX
  method to registers, I think. _HPX method returns the value to write
  to registers.
    - Cache-line size Register
    - Latency timer Register
    - Enable SERR Bit/Enable PERR Bit in Device Control Register
    - Uncorrectable Error Mask Register
    - Uncorrectable Error Severity Register
    - Correctable Error Mask Register
    - Advanced Error Capabilities and Control Register
    - Device Control Register
    - Link Control Register
    - Secondary Uncorrectable Error Severity Register
    - Secondary Uncorrectable Error Mask Register
    - Device Control 2 Register
    - Link Control 2 Register

- The following resister should be configured to "0".
    - PME Enable Bit/PME Status Bit in PCI Power Management
      Control/Status Register

I would like to discuss this logic more.

Thanks.

--
Yuji Shimada


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yes, when a domain is destroyed, the place where I do FLR is late. Thanks for
pointing out this.
Looks the situation here is a little tricky. I''m trying to find a
better place.

When a device is not assigned to other domain, since Dom0 might dynamically
rebind the device to another driver rather than pciback, "completely
deassign the devices from the IOMMU" may not be a good idea?

For the old xend running on a new hypervisor, FLR is actually skipped.
I''m not sure if this is unacceptable. I''d like to hear more
comments.

Thanks,
-- Dexuan


-----Original Message-----
From: Espen Skoglund [mailto:espen.skoglund@netronome.com] 
Sent: 2008年7月15日 1:52
To: Cui, Dexuan
Cc: Keir Fraser; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] Improve the current FLR logic

Maybe I''ve got this wrong, but it looks like you''re doing the
FLR
after the device has been deassigned (i.e., given back to dom0).
Shouldn''t you do the FLR before you actually deassign the device
instead?  If the device is currently set up for doing DMA transactions
to host memory, the pending transactions will end up in dom0''s memory
space.  This is fine if GMFN == MFN since the guest memory has still
not been released, but for GMFN != MFN you''ll end up corrupting
arbitrary dom0 memory.

The right procedure for deassigning devices should be:

  - Revoke device resources.
  - FLR.
  - Reassign device to dom0.

Likewise, for assigning devices we should do:

  - FLR.
  - Assign device to guest.
  - Grant device resources to guest.

Another option is to completely deassign the devices from the IOMMU
rather than reassigning them to dom0.  Devices bound to pciback need
not be assigned to dom0''s IOMMU tables anyway.

Further, your patch probably breaks things when running old dom0/xend
on a new hypervisor since you''ll end up not doing any FLR at all.  I
recently experienced the same thing with some of my own PCI cleanup
patches.

	eSk


[Dexuan Cui]
> Hi, all,
> The attached patches try to improve the current FLR logic.
> The idea is: removing the FLR logic from hypervisor and adding the
> improved logic in Control Panel.
> The current FLR logic in hypervisor has some issues: 1) Dstate
> transition is not guaranteed to properly clear the device state; 2) the
> current code for PCIe FLR is actually buggy: PCI_EXP_DEVSTA_TRPND
> doesn''t mean the completion of FLR; according to the PCIe spec,
after
> issuing FLR, we should wait at least 100ms.
> To make it easier to improve the FLR logic, and to make the hypervisor
> thin, I think we might as well move the logic to Control Panel for the
> time being. In the long run, the essential logic may be implemented in
> the pciback driver of Dom0 instead.
> [...]
> I made some tests on my hosts. Looks the patches work well.
> I''d like to ask for your comments, and test feedbacks. Thank you
> very much!
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Thanks for your comments! 

For case 2) or 4), I think we might as well force the domain config file
specifying all the related devices.

xm_pci_list_assignable_devices() does access to sysfs and invoke hypercall
directly.
With respect to xmlrpc, I''m trying to move most of the function to
xend.

I think xm_pci_list_assignable_devices only considers iommu devices. Maybe we
can extend  it to non-iommu devices in future.

Thanks,
-- Dexuan


-----Original Message-----
From: Yosuke Iwamatsu [mailto:y-iwamatsu@ab.jp.nec.com] 
Sent: 2008年7月15日 12:06
To: Cui, Dexuan
Cc: Keir Fraser; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] Improve the current FLR logic

Hi,

Cui, Dexuan wrote:
> [PATCH3] do_FLR_in_control_panel.patch:  the improved FLR logic in
> Control Panel: 1) If the device is PCIe endpoint and supports PCIe FLR,
> we use that; 2) Else, if the device is PCIe endpoint, and all functions
> on the device are assigned to the same guest, we use the immediate
> parent bus''s Secondary Bus Reset to reset all functions of the
device
> (here, actually we require all the functions of the device be assigned
> to the same guest); 3) Else, if the device is PCI endpoint and is on a
> host bus (e.g. integrated devices) and if the device supports PCI
> Advanced Capabilities, we use that for FLR; 4) Else, we use the
> Secondary Bus Reset (if the PCI device is behind a PCI/PCI-X bridge,
> then all devices behind the uppermost such PCI/PCI-X bridge above this
> device must be co-assigned)
In case 2) or 4), I think multfunction devices or sibling devices need
not be actually assigned to the same guest, if they are just owned by
pciback and not used by other guests.
> [PATCH4] list_assignable_devices.patch: a command "xm
> pci-list-assignable-devices" which can list all the assignable devices
> in the system. It is useful for end users.
It looks like xm_pci_list_assignable_devices() directly accesses to
sysfs and invokes hypercall without going through xend. Since xm
commands can be executed by a remote server through xmlrpc, most of
this function should be implemented in xend.
Also please note that test_assign_device() excludes the case of
non-iommu device assignment for pv.

Thanks,
-- Yosuke Iwamatsu

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Thanks for the comments!
Yes, the current simple save/restore has issues, and you have pointed some.
Let me think over all the issues and work out a better solution.

Thanks,
-- Dexuan


-----Original Message-----
From: Yuji Shimada [mailto:shimada-yxb@necst.nec.co.jp] 
Sent: 2008年7月17日 17:34
To: Cui, Dexuan; Keir Fraser; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] Improve the current FLR logic

On Sat, 12 Jul 2008 20:37:34 +0800
"Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> I''d like to ask for your comments, and test feedbacks. Thank you
very
> much!
There is a problem with the device reconfiguring logic. xend saves all
Configuration Register''s values before reset. And xend writes the
values to the registers after reset. This means the resister''s values
which are configured by guest software are restore. Guest software
setting should be cleared.

I think the following Configuration Resister should be reconfigured to
correct values after reset. And other registers should not be
reconfigured after reset if there is no reason.

- It is necessary to write the base address of the resource allocated
  by the dom0 kernel to the following resister.
    - Base Address Register

- When dom0 starts, the values configured by firmware should be saved,
  and the values should be written to the following resisters after
  reset. The reason is that firmware configures them to archive system
  specific functions. Especially, the registers relating error reporting
  are configured to collect error information. If this configured values
  is changed, some functions might be lost.
  Instead of save/restore, it is good way to write the value from _HPX
  method to registers, I think. _HPX method returns the value to write
  to registers.
    - Cache-line size Register
    - Latency timer Register
    - Enable SERR Bit/Enable PERR Bit in Device Control Register
    - Uncorrectable Error Mask Register
    - Uncorrectable Error Severity Register
    - Correctable Error Mask Register
    - Advanced Error Capabilities and Control Register
    - Device Control Register
    - Link Control Register
    - Secondary Uncorrectable Error Severity Register
    - Secondary Uncorrectable Error Mask Register
    - Device Control 2 Register
    - Link Control 2 Register

- The following resister should be configured to "0".
    - PME Enable Bit/PME Status Bit in PCI Power Management
      Control/Status Register

I would like to discuss this logic more.

Thanks.

--
Yuji Shimada


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Cui, Dexuan wrote:
> Thanks for your comments! 
> 
> For case 2) or 4), I think we might as well force the domain config file
specifying all the related devices.
Specifying all the related devices means allowing the domain to use them
all, but there may be a case that people want to limit devices available
to guests.
(Suppose there is a device that has several functions without FLR and
the administrator wants to let the guest domain use only one function,
for example.)
> 
> xm_pci_list_assignable_devices() does access to sysfs and invoke hypercall
directly.
> With respect to xmlrpc, I''m trying to move most of the function to
xend.
> 
> I think xm_pci_list_assignable_devices only considers iommu devices. Maybe
we can extend  it to non-iommu devices in future.
> 
Thank you,
-- Yosuke

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

What I was thinking of with deassigning it completely was that pciback
performs a deassign when it is binds to a device, and an assign to
dom0 when it unbinds a device.  The latter part is not strictly
necessary though, since Linux will also unregister the device from Xen
when pciback performs unbind.  This implies a deassign and a
subsequent register and assign to dom0 once the device is bound to
another driver.

Doing a complete deassing instead of reassign to dom0 would ensure
that the whole FLR/deassign order problem is circumvented.

	eSk



-----Original Message-----
From: "Cui, Dexuan" <dexuan.cui@intel.com>
Subject: RE: [Xen-devel] [PATCH] Improve the current FLR logic
Date: Thu, 17 Jul 2008 18:21:36 +0800

Yes, when a domain is destroyed, the place where I do FLR is late.
Thanks for pointing out this.
Looks the situation here is a little tricky. I''m trying to find a
better
place.

When a device is not assigned to other domain, since Dom0 might
dynamically rebind the device to another driver rather than pciback,
"completely deassign the devices from the IOMMU" may not be a good
idea?

For the old xend running on a new hypervisor, FLR is actually skipped.
I''m not sure if this is unacceptable. I''d like to hear more
comments.

Thanks,
-- Dexuan


-----Original Message-----
From: Espen Skoglund [mailto:espen.skoglund@netronome.com]=20
Sent: 2008=C4=EA7=D4=C215=C8=D5 1:52
To: Cui, Dexuan
Cc: Keir Fraser; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] Improve the current FLR logic

Maybe I''ve got this wrong, but it looks like you''re doing the
FLR
after the device has been deassigned (i.e., given back to dom0).
Shouldn''t you do the FLR before you actually deassign the device
instead?  If the device is currently set up for doing DMA transactions
to host memory, the pending transactions will end up in dom0''s memory
space.  This is fine if GMFN =3D=3D MFN since the guest memory has still
not been released, but for GMFN !=3D MFN you''ll end up corrupting
arbitrary dom0 memory.

The right procedure for deassigning devices should be:

  - Revoke device resources.
  - FLR.
  - Reassign device to dom0.

Likewise, for assigning devices we should do:

  - FLR.
  - Assign device to guest.
  - Grant device resources to guest.

Another option is to completely deassign the devices from the IOMMU
rather than reassigning them to dom0.  Devices bound to pciback need
not be assigned to dom0''s IOMMU tables anyway.

Further, your patch probably breaks things when running old dom0/xend
on a new hypervisor since you''ll end up not doing any FLR at all.  I
recently experienced the same thing with some of my own PCI cleanup
patches.

	eSk


[Dexuan Cui]
> Hi, all,
> The attached patches try to improve the current FLR logic.
> The idea is: removing the FLR logic from hypervisor and adding the
> improved logic in Control Panel.
> The current FLR logic in hypervisor has some issues: 1) Dstate
> transition is not guaranteed to properly clear the device state; 2) the
> current code for PCIe FLR is actually buggy: PCI_EXP_DEVSTA_TRPND
> doesn''t mean the completion of FLR; according to the PCIe spec,
after
> issuing FLR, we should wait at least 100ms.
> To make it easier to improve the FLR logic, and to make the hypervisor
> thin, I think we might as well move the logic to Control Panel for the
> time being. In the long run, the essential logic may be implemented in
> the pciback driver of Dom0 instead.
> [...]
> I made some tests on my hosts. Looks the patches work well.
> I''d like to ask for your comments, and test feedbacks. Thank you
> very much!
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yosuke Iwamatsu wrote:
> Cui, Dexuan wrote:
>> Thanks for your comments!
>> 
>> For case 2) or 4), I think we might as well force the domain config
>> file specifying all the related devices. 
> 
> Specifying all the related devices means allowing the domain to use
> them all, but there may be a case that people want to limit devices
> available to guests.
> (Suppose there is a device that has several functions without FLR and
> the administrator wants to let the guest domain use only one function,
> for example.)
Oh, I see.  I should relax the checking here.
Thank you, Yosuke!

Thanks,
-- Dexuan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Will this method impact PV domU when iommu_pv is not specified?

In fact, I have a look on the tools\python\xen\xend\server\pciif.py, I
noticed it is much bigger than other file (like tpmif.py or vscsiif.py)
in the same directory. and some pci logic in XendDomainInfo.py also. So
are there any rules when decide a function should be placed in control
panel or in backend? 

As for this order issue, I think it is ok for HVM domain, since
domain_destroy should happen after control panel do the cleanup work (We
may need do that before qemu is destroed). But I''m not sure if PV
domain
will have such opporunity.

Thanks
Yunhong Jiang

Espen Skoglund <mailto:espen.skoglund@netronome.com>
wrote:
> What I was thinking of with deassigning it completely was that pciback
> performs a deassign when it is binds to a device, and an assign to
> dom0 when it unbinds a device.  The latter part is not strictly
> necessary though, since Linux will also unregister the device from Xen
> when pciback performs unbind.  This implies a deassign and a
> subsequent register and assign to dom0 once the device is bound to
another
> driver. 
> 
> Doing a complete deassing instead of reassign to dom0 would ensure
> that the whole FLR/deassign order problem is circumvented.
> 
> 	eSk
> 
> 
> 
> -----Original Message-----
> From: "Cui, Dexuan" <dexuan.cui@intel.com>
> Subject: RE: [Xen-devel] [PATCH] Improve the current FLR logic
> Date: Thu, 17 Jul 2008 18:21:36 +0800
> 
> Yes, when a domain is destroyed, the place where I do FLR is late.
Thanks
> for pointing out this. Looks the situation here is a little tricky.
I''m
> trying to 
> find a better
> place.
> 
> When a device is not assigned to other domain, since Dom0 might
> dynamically rebind the device to another driver rather than pciback,
> "completely deassign the devices from the IOMMU" may not be a
> good idea?
> 
> For the old xend running on a new hypervisor, FLR is actually skipped.
> I''m not sure if this is unacceptable. I''d like to hear
more comments.
> 
> Thanks,
> -- Dexuan
> 
> 
> -----Original Message-----
> From: Espen Skoglund [mailto:espen.skoglund@netronome.com]=20
> Sent: 2008=C4=EA7=D4=C215=C8=D5 1:52
> To: Cui, Dexuan
> Cc: Keir Fraser; xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] [PATCH] Improve the current FLR logic
> 
> Maybe I''ve got this wrong, but it looks like you''re doing
the FLR
> after the device has been deassigned (i.e., given back to dom0).
> Shouldn''t you do the FLR before you actually deassign the device
> instead?  If the device is currently set up for doing DMA transactions
> to host memory, the pending transactions will end up in dom0''s
memory
> space.  This is fine if GMFN =3D=3D MFN since the guest memory
> has still
> not been released, but for GMFN !=3D MFN you''ll end up corrupting
arbitrary
> dom0 memory. 
> 
> The right procedure for deassigning devices should be:
> 
>  - Revoke device resources.
>  - FLR.
>  - Reassign device to dom0.
> 
> Likewise, for assigning devices we should do:
> 
>  - FLR.
>  - Assign device to guest.
>  - Grant device resources to guest.
> 
> Another option is to completely deassign the devices from the IOMMU
> rather than reassigning them to dom0.  Devices bound to pciback need
> not be assigned to dom0''s IOMMU tables anyway.
> 
> Further, your patch probably breaks things when running old dom0/xend
> on a new hypervisor since you''ll end up not doing any FLR at all. 
I
> recently experienced the same thing with some of my own PCI cleanup
patches.
> 
> 	eSk
> 
> 
> [Dexuan Cui]
>> Hi, all,
>> The attached patches try to improve the current FLR logic.
>> The idea is: removing the FLR logic from hypervisor and adding the
>> improved logic in Control Panel.
> 
>> The current FLR logic in hypervisor has some issues: 1) Dstate
>> transition is not guaranteed to properly clear the device state; 2) the
>> current code for PCIe FLR is actually buggy: PCI_EXP_DEVSTA_TRPND
>> doesn''t mean the completion of FLR; according to the PCIe
spec, after
>> issuing FLR, we should wait at least 100ms.
> 
>> To make it easier to improve the FLR logic, and to make the
hypervisor
>> thin, I think we might as well move the logic to Control Panel for
the
>> time being. In the long run, the essential logic may be implemented
in
>> the pciback driver of Dom0 instead.
> 
>> [...]
> 
>> I made some tests on my hosts. Looks the patches work well.
> 
>> I''d like to ask for your comments, and test feedbacks. Thank
you
>> very much!
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yuji Shimada wrote:
> On Sat, 12 Jul 2008 20:37:34 +0800
> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> 
>> I''d like to ask for your comments, and test feedbacks. Thank
you very
>> much!
> 
> There is a problem with the device reconfiguring logic. xend saves all
> Configuration Register''s values before reset. And xend writes the
> values to the registers after reset. This means the resister''s
values
> which are configured by guest software are restore. Guest software
> setting should be cleared.
Now xend saves and restores all the 256-byte space -- this is not
suitable as you pointed.
How about only saving/restoring the header (the first 64-byte)? 
I think this may be a good idea. 
I tend to make the least change to the current code. :-)


Thanks,
-- Dexuan
> 
> I think the following Configuration Resister should be reconfigured to
> correct values after reset. And other registers should not be
> reconfigured after reset if there is no reason.
> 
> - It is necessary to write the base address of the resource allocated
>   by the dom0 kernel to the following resister.
>     - Base Address Register
> 
> - When dom0 starts, the values configured by firmware should be saved,
>   and the values should be written to the following resisters after
>   reset. The reason is that firmware configures them to archive system
>   specific functions. Especially, the registers relating error
>   reporting are configured to collect error information. If this
>   configured values is changed, some functions might be lost.
>   Instead of save/restore, it is good way to write the value from _HPX
>   method to registers, I think. _HPX method returns the value to write
>   to registers.
>     - Cache-line size Register
>     - Latency timer Register
>     - Enable SERR Bit/Enable PERR Bit in Device Control Register
>     - Uncorrectable Error Mask Register
>     - Uncorrectable Error Severity Register
>     - Correctable Error Mask Register
>     - Advanced Error Capabilities and Control Register
>     - Device Control Register
>     - Link Control Register
>     - Secondary Uncorrectable Error Severity Register
>     - Secondary Uncorrectable Error Mask Register
>     - Device Control 2 Register
>     - Link Control 2 Register
> 
> - The following resister should be configured to "0".
>     - PME Enable Bit/PME Status Bit in PCI Power Management
>       Control/Status Register
> 
> I would like to discuss this logic more.
> 
> Thanks.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 18 Jul 2008 14:37:30 +0800
"Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> Now xend saves and restores all the 256-byte space -- this is not
> suitable as you pointed.
> How about only saving/restoring the header (the first 64-byte)? 
I think following registers in PCI Express Capability Structure should be
restored. The reason is that host firmware might configure them.

     - Uncorrectable Error Mask Register
     - Uncorrectable Error Severity Register
     - Correctable Error Mask Register
     - Advanced Error Capabilities and Control Register
     - Device Control Register
     - Link Control Register
     - Secondary Uncorrectable Error Severity Register
     - Secondary Uncorrectable Error Mask Register
     - Device Control 2 Register
     - Link Control 2 Register

What do you think about following method?

     1. create the table, and fill offset, size, etc. of registers to
        save/restore into the table.

     2. save/restore registers based on the table.

Thanks.

--
Yuji Shimada

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yuji Shimada wrote:
> On Fri, 18 Jul 2008 14:37:30 +0800
> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> 
>> Now xend saves and restores all the 256-byte space -- this is not
>> suitable as you pointed. How about only saving/restoring the header
>> (the first 64-byte)? 
> 
> I think following registers in PCI Express Capability Structure
> should be restored. The reason is that host firmware might configure
> them. 
> 
>      - Uncorrectable Error Mask Register
>      - Uncorrectable Error Severity Register
>      - Correctable Error Mask Register
>      - Advanced Error Capabilities and Control Register
>      - Device Control Register
>      - Link Control Register
>      - Secondary Uncorrectable Error Severity Register
>      - Secondary Uncorrectable Error Mask Register
>      - Device Control 2 Register
>      - Link Control 2 Register
Hi Yuji,
Thanks for your advice!
To restore the values of the PCIe registers to the original values the
firmware (and Dom0?) configues, we need to store the original values
somewhere. 
Maybe we can restore the info into xenstore?
> What do you think about following method?
> 
>      1. create the table, and fill offset, size, etc. of registers to
>         save/restore into the table.
> 
>      2. save/restore registers based on the table.
This sounds nice.
Where should we store the table?

Thanks,
-- Dexuan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 18 Jul 2008 18:43:49 +0800
"Cui, Dexuan" <dexuan.cui@intel.com>
wrote:
> Hi Yuji,
> Thanks for your advice!
> To restore the values of the PCIe registers to the original values the
> firmware (and Dom0?) configues, we need to store the original values
> somewhere. 
> Maybe we can restore the info into xenstore?
Hi Cui,
Xenstore is OK, but pciback is better place, I think.
Because device driver should configure devices.
If we do it in pciback, the coding might be difficult. But the
whole architecture will be simple.

I think the following saving/restoring timing is good.
  - saving    : after pciback bind device
  - restoring : after xend writes SBDF into xenstore
                before pciback removes backend device
>> What do you think about following method?
>> 
>>      1. create the table, and fill offset, size, etc. of registers to
>>         save/restore into the table.
>> 
>>      2. save/restore registers based on the table.
> This sounds nice.
> Where should we store the table?
I don''t think device specific tables are required.
So we only have to create one table.
xend (or pciback if you implement saving/restoring in pciback) is good
place, I think.

Thanks.

--
Yuji Shimada

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On 22/7/08 11:16, "Yuji Shimada" <shimada-yxb@necst.nec.co.jp>
wrote:
> Xenstore is OK, but pciback is better place, I think.
> Because device driver should configure devices.
> If we do it in pciback, the coding might be difficult. But the
> whole architecture will be simple.
I find the model of pciback owning pass-thru''ed devices, configuring
and
resetting them, and ultimately handing them off to domU guests via the PV
front-back interface or via qemu-dm emulation, is quite attractive at least
conceptually. As you say, I''m not sure what devils may lie in the
details.

Still, it seems nicer than hacking logic into xend. :-)

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yuji Shimada wrote:
> On Fri, 18 Jul 2008 18:43:49 +0800
> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>> Hi Yuji,
>> Thanks for your advice!
>> To restore the values of the PCIe registers to the original values
>> the firmware (and Dom0?) configues, we need to store the original
>> values somewhere. Maybe we can restore the info into xenstore?
> 
> Hi Cui,
> Xenstore is OK, but pciback is better place, I think.
> Because device driver should configure devices.
I think xenstore seems to be a little ugly place to remember the proper
values of all or part of registers of all the devices in the system.
I agree pciback is a good place.
> If we do it in pciback, the coding might be difficult. But the
> whole architecture will be simple.
Yes. Now I realize pciback seems to be the best place to do most of the
FLR logic.
But this needs modify pciback and add interfaces for Control Panel.
I''m not sure the changes can be small enought to be in 3.3.
> 
> I think the following saving/restoring timing is good.
>   - saving    : after pciback bind device
>   - restoring : after xend writes SBDF into xenstore
>                 before pciback removes backend device
> 
>>> What do you think about following method?
>>> 
>>>      1. create the table, and fill offset, size, etc. of registers
>>>         to save/restore into the table.
>>> 
>>>      2. save/restore registers based on the table.
>> This sounds nice.
>> Where should we store the table?
> 
> I don''t think device specific tables are required.
> So we only have to create one table.
> xend (or pciback if you implement saving/restoring in pciback) is good
> place, I think.
> 
Thanks,
-- Dexuan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Wed, 23 Jul 2008 11:16:31 +0800
"Cui, Dexuan" <dexuan.cui@intel.com>
wrote:
> I think xenstore seems to be a little ugly place to remember the proper
> values of all or part of registers of all the devices in the system.
> I agree pciback is a good place.
> Yes. Now I realize pciback seems to be the best place to do most of the
> FLR logic.
> But this needs modify pciback and add interfaces for Control Panel.
> I''m not sure the changes can be small enought to be in 3.3.
Hi Cui,

What do you think about the following idea?
  - For 3.3, modify xend to have limited saving/restoring and resetting
    functions.
  - For 3.4, modify pciback to have proper saving/restoring and
    resetting functions.

The purpose of saving is to save the values configured by firmware. So
the best timing of saving the value is when dom0 starts. But if saving
the values when dom0 starts, pciback is the best place to save/restore.
If pciback saves/restores the values, we need many codings. So it is
difficult to achieve it in 3.3.

So for 3.3, there is a way that saving the value just before resetting.
It is possible xend saves/restores the value, I think. In this case,
xend don''t need saving the values in xenstore. Because interval between
saving and restoring is small.

My idea is summarized as follows:

Xen    Timing              Where      Difficulty
3.3    before resetting    xend       easy
3.4    when Dom0 starts    pciback    hard

Thanks.

--
Yuji Shimada


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

> Yuji Shimada wrote:
> > On Wed, 23 Jul 2008 11:16:31 +0800
> > "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
> >> I think xenstore seems to be a little ugly place to remember the
> >> proper values of all or part of registers of all the devices in
the
> >> system. 
> >> I agree pciback is a good place.
> >> Yes. Now I realize pciback seems to be the best place to do most
of
> >> the FLR logic. But this needs modify pciback and add interfaces
for
> >> Control Panel. I''m not sure the changes can be small
enought to be
> >> in 3.3. 
> > 
> > Hi Cui,
> > 
> > What do you think about the following idea?
> >   - For 3.3, modify xend to have limited saving/restoring and
> >     resetting functions.
> >   - For 3.4, modify pciback to have proper saving/restoring and
> >     resetting functions.
> Hi Yuji,
> Thanks very much for the suggestions!
> I agree.
> 
> > The purpose of saving is to save the values configured by firmware. So
> > the best timing of saving the value is when dom0 starts. But if saving
> > the values when dom0 starts, pciback is the best place to
> > save/restore. If pciback saves/restores the values, we need many
> > codings. So it is difficult to achieve it in 3.3.
> > 
> > So for 3.3, there is a way that saving the value just before
> > resetting. It is possible xend saves/restores the value, I think. In
> 
> In current code of xend,
> The steps are:
> 1) save the current values of the configuration space registers
> 2) do_FLR
> 3) restore the values of the registers
> 
> "a way that saving the value just before resetting." -- do you
mean the
> same thing? 
> 
Hi Cui,

Yes, I mean the same thing. But in current code of xend, xend saves all
Configuration Registers. The only registers I suggested before should be
saved. Small modification is needed.
> > this case, xend don''t need saving the values in xenstore.
Because
> > interval between saving and restoring is small.
> > 
> > My idea is summarized as follows:
> > 
> > Xen    Timing              Where      Difficulty
> > 3.3    before resetting    xend       easy
> > 3.4    when Dom0 starts    pciback    hard
> > 
> I exactly agree with you.
> 
Thanks.
--
Yuji Shimada

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yuji Shimada wrote:
>> Yuji Shimada wrote:
>>> On Wed, 23 Jul 2008 11:16:31 +0800
>>> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>>>> I think xenstore seems to be a little ugly place to remember
the
>>>> proper values of all or part of registers of all the devices in
>>>> the system. I agree pciback is a good place.
>>>> Yes. Now I realize pciback seems to be the best place to do
most of
>>>> the FLR logic. But this needs modify pciback and add interfaces
for
>>>> Control Panel. I''m not sure the changes can be small
enought to be
>>>> in 3.3.
>>> 
>>> Hi Cui,
>>> 
>>> What do you think about the following idea?
>>>   - For 3.3, modify xend to have limited saving/restoring and    
>>> resetting functions. 
>>>   - For 3.4, modify pciback to have proper saving/restoring and
>>>     resetting functions.
>> Hi Yuji,
>> Thanks very much for the suggestions!
>> I agree.
>> 
>>> The purpose of saving is to save the values configured by firmware.
>>> So the best timing of saving the value is when dom0 starts. But if
>>> saving the values when dom0 starts, pciback is the best place to
>>> save/restore. If pciback saves/restores the values, we need many
>>> codings. So it is difficult to achieve it in 3.3.
>>> 
>>> So for 3.3, there is a way that saving the value just before
>>> resetting. It is possible xend saves/restores the value, I think.
In
>> 
>> In current code of xend,
>> The steps are:
>> 1) save the current values of the configuration space registers 2)
>> do_FLR 3) restore the values of the registers
>> 
>> "a way that saving the value just before resetting." -- do
you mean
>> the same thing? 
>> 
> 
> Hi Cui,
> 
> Yes, I mean the same thing. But in current code of xend, xend saves
> all Configuration Registers. The only registers I suggested before
> should be saved. Small modification is needed.
It sounds good. Namely, we need to save/restore the following registers
for now:

    - Base Address Registers
    - Cache-line size Register
    - Latency timer Register
    - Enable SERR Bit/Enable PERR Bit in Device Control Register
    - Uncorrectable Error Mask Register
    - Uncorrectable Error Severity Register
    - Correctable Error Mask Register
    - Advanced Error Capabilities and Control Register
    - Device Control Register
    - Link Control Register
    - Secondary Uncorrectable Error Severity Register
    - Secondary Uncorrectable Error Mask Register
    - Device Control 2 Register
    - Link Control 2 Register
    - The following resister should be configured to "0".
          - PME Enable Bit/PME Status Bit in PCI Power Management
            Control/Status Register

However, I think maybe the modification is not small enough because
1) we need to save each registers one by one using Python script in
xend, and later restore each registers respectively one by one;
2) we should handle bridge in some cases, so we need to distinguish
bridges from regular devices since the register layouts are different;
3) Some of the registers you listed are inside the extended PCIe space,
so we need detect if a device/bride has the PCIe capability? And find
each capability/save the register;
4) xend uses the sys filesystem to access the registers. For the case of
PCIe registers, when Dom0 is configured with/without PCIe support (by
default, it''s "without" now), we should detect and treat it
differently?

Acutually looks the save/restore-all-the-256-byte idea (which was in
hypervisor and is in xend now) works very well for quite a long time,
and no actual issue is reported as far as I know. Since it looks very
difficult to do things perfectly for now and we''ll improve them by
changing pciback in the long run, maybe keeping the current simple
method is acceptable? :-)

Thanks,
-- Dexuan
> 
>>> this case, xend don''t need saving the values in xenstore.
Because
>>> interval between saving and restoring is small.
>>> 
>>> My idea is summarized as follows:
>>> 
>>> Xen    Timing              Where      Difficulty
>>> 3.3    before resetting    xend       easy
>>> 3.4    when Dom0 starts    pciback    hard
>>> 
>> I exactly agree with you.
>> 
> 
> Thanks.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Thu, 24 Jul 2008 18:19:24 +0800
"Cui, Dexuan" <dexuan.cui@intel.com>
wrote:
> It sounds good. Namely, we need to save/restore the following registers
> for now:
> 
>     - Base Address Registers
>     - Cache-line size Register
>     - Latency timer Register
>     - Enable SERR Bit/Enable PERR Bit in Device Control Register
>     - Uncorrectable Error Mask Register
>     - Uncorrectable Error Severity Register
>     - Correctable Error Mask Register
>     - Advanced Error Capabilities and Control Register
>     - Device Control Register
>     - Link Control Register
>     - Secondary Uncorrectable Error Severity Register
>     - Secondary Uncorrectable Error Mask Register
>     - Device Control 2 Register
>     - Link Control 2 Register
>     - The following resister should be configured to "0".
>           - PME Enable Bit/PME Status Bit in PCI Power Management
>             Control/Status Register
> 
> However, I think maybe the modification is not small enough because
> 1) we need to save each registers one by one using Python script in
> xend, and later restore each registers respectively one by one;
> 2) we should handle bridge in some cases, so we need to distinguish
> bridges from regular devices since the register layouts are different;
> 3) Some of the registers you listed are inside the extended PCIe space,
> so we need detect if a device/bride has the PCIe capability? And find
> each capability/save the register;
> 4) xend uses the sys filesystem to access the registers. For the case of
> PCIe registers, when Dom0 is configured with/without PCIe support (by
> default, it''s "without" now), we should detect and treat
it differently?
> 
> Acutually looks the save/restore-all-the-256-byte idea (which was in
> hypervisor and is in xend now) works very well for quite a long time,
> and no actual issue is reported as far as I know. Since it looks very
> difficult to do things perfectly for now and we''ll improve them by
> changing pciback in the long run, maybe keeping the current simple
> method is acceptable? :-)
Hi Cui,
I find that the modification is not small enough. If we improve them,
it won''t be on time for 3.3. So we have no choice but to accept the
current method.

The registers other than above-mentioned should be reset. Because
guest software may configure the registers incorrect values.
My idea has a effect in abnormal case only, and it won''t occur when
guest software works well. So I think nobody reports about it so far.

I hope pciback will have better saving/restoring method in 3.4.

Thanks.
--
Yuji Shimada

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yuji Shimada wrote:
> On Thu, 24 Jul 2008 18:19:24 +0800
> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>> It sounds good. Namely, we need to save/restore the following
>> registers for now: 
>> 
>>     - Base Address Registers
>>     - Cache-line size Register
>>     - Latency timer Register
>>     - Enable SERR Bit/Enable PERR Bit in Device Control Register
>>     - Uncorrectable Error Mask Register
>>     - Uncorrectable Error Severity Register
>>     - Correctable Error Mask Register
>>     - Advanced Error Capabilities and Control Register
>>     - Device Control Register
>>     - Link Control Register
>>     - Secondary Uncorrectable Error Severity Register
>>     - Secondary Uncorrectable Error Mask Register
>>     - Device Control 2 Register
>>     - Link Control 2 Register
>>     - The following resister should be configured to "0".
>>           - PME Enable Bit/PME Status Bit in PCI Power Management
>>             Control/Status Register
>> 
>> However, I think maybe the modification is not small enough because
>> 1) we need to save each registers one by one using Python script in
>> xend, and later restore each registers respectively one by one;
>> 2) we should handle bridge in some cases, so we need to distinguish
>> bridges from regular devices since the register layouts are
>> different; 3) Some of the registers you listed are inside the
>> extended PCIe space, so we need detect if a device/bride has the
>> PCIe capability? And find each capability/save the register;
>> 4) xend uses the sys filesystem to access the registers. For the
>> case of PCIe registers, when Dom0 is configured with/without PCIe
>> support (by default, it''s "without" now), we should
detect and treat
>> it differently? 
>> 
>> Acutually looks the save/restore-all-the-256-byte idea (which was in
>> hypervisor and is in xend now) works very well for quite a long time,
>> and no actual issue is reported as far as I know. Since it looks very
>> difficult to do things perfectly for now and we''ll improve
them by
>> changing pciback in the long run, maybe keeping the current simple
>> method is acceptable? :-)
> 
> Hi Cui,
> I find that the modification is not small enough. If we improve them,
> it won''t be on time for 3.3. So we have no choice but to accept
the
> current method.
> 
> The registers other than above-mentioned should be reset. Because
> guest software may configure the registers incorrect values.
> My idea has a effect in abnormal case only, and it won''t occur
when
> guest software works well. So I think nobody reports about it so far.
> 
> I hope pciback will have better saving/restoring method in 3.4.
> 
Hi Yuji,
I agree. Thanks for all the comments!

Thanks,
-- Dexuan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Dexuan,

May I know the current development stage of FLR in pciback driver?

Thanks,
Neo

On Thu, Jul 24, 2008 at 3:19 AM, Cui, Dexuan <dexuan.cui@intel.com>
wrote:
> Yuji Shimada wrote:
>>> Yuji Shimada wrote:
>>>> On Wed, 23 Jul 2008 11:16:31 +0800
>>>> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>>>>> I think xenstore seems to be a little ugly place to
remember the
>>>>> proper values of all or part of registers of all the
devices in
>>>>> the system. I agree pciback is a good place.
>>>>> Yes. Now I realize pciback seems to be the best place to do
most of
>>>>> the FLR logic. But this needs modify pciback and add
interfaces for
>>>>> Control Panel. I''m not sure the changes can be
small enought to be
>>>>> in 3.3.
>>>>
>>>> Hi Cui,
>>>>
>>>> What do you think about the following idea?
>>>>   - For 3.3, modify xend to have limited saving/restoring and
>>>> resetting functions.
>>>>   - For 3.4, modify pciback to have proper saving/restoring and
>>>>     resetting functions.
>>> Hi Yuji,
>>> Thanks very much for the suggestions!
>>> I agree.
>>>
>>>> The purpose of saving is to save the values configured by
firmware.
>>>> So the best timing of saving the value is when dom0 starts. But
if
>>>> saving the values when dom0 starts, pciback is the best place
to
>>>> save/restore. If pciback saves/restores the values, we need
many
>>>> codings. So it is difficult to achieve it in 3.3.
>>>>
>>>> So for 3.3, there is a way that saving the value just before
>>>> resetting. It is possible xend saves/restores the value, I
think. In
>>>
>>> In current code of xend,
>>> The steps are:
>>> 1) save the current values of the configuration space registers 2)
>>> do_FLR 3) restore the values of the registers
>>>
>>> "a way that saving the value just before resetting." --
do you mean
>>> the same thing?
>>>
>>
>> Hi Cui,
>>
>> Yes, I mean the same thing. But in current code of xend, xend saves
>> all Configuration Registers. The only registers I suggested before
>> should be saved. Small modification is needed.
>
> It sounds good. Namely, we need to save/restore the following registers
> for now:
>
>    - Base Address Registers
>    - Cache-line size Register
>    - Latency timer Register
>    - Enable SERR Bit/Enable PERR Bit in Device Control Register
>    - Uncorrectable Error Mask Register
>    - Uncorrectable Error Severity Register
>    - Correctable Error Mask Register
>    - Advanced Error Capabilities and Control Register
>    - Device Control Register
>    - Link Control Register
>    - Secondary Uncorrectable Error Severity Register
>    - Secondary Uncorrectable Error Mask Register
>    - Device Control 2 Register
>    - Link Control 2 Register
>    - The following resister should be configured to "0".
>          - PME Enable Bit/PME Status Bit in PCI Power Management
>            Control/Status Register
>
> However, I think maybe the modification is not small enough because
> 1) we need to save each registers one by one using Python script in
> xend, and later restore each registers respectively one by one;
> 2) we should handle bridge in some cases, so we need to distinguish
> bridges from regular devices since the register layouts are different;
> 3) Some of the registers you listed are inside the extended PCIe space,
> so we need detect if a device/bride has the PCIe capability? And find
> each capability/save the register;
> 4) xend uses the sys filesystem to access the registers. For the case of
> PCIe registers, when Dom0 is configured with/without PCIe support (by
> default, it''s "without" now), we should detect and treat
it differently?
>
> Acutually looks the save/restore-all-the-256-byte idea (which was in
> hypervisor and is in xend now) works very well for quite a long time,
> and no actual issue is reported as far as I know. Since it looks very
> difficult to do things perfectly for now and we''ll improve them by
> changing pciback in the long run, maybe keeping the current simple
> method is acceptable? :-)
>
> Thanks,
> -- Dexuan
>>
>>>> this case, xend don''t need saving the values in
xenstore. Because
>>>> interval between saving and restoring is small.
>>>>
>>>> My idea is summarized as follows:
>>>>
>>>> Xen    Timing              Where      Difficulty
>>>> 3.3    before resetting    xend       easy
>>>> 3.4    when Dom0 starts    pciback    hard
>>>>
>>> I exactly agree with you.
>>>
>>
>> Thanks.
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>


-- 
I would remember that if researchers were not ambitious
probably today we haven''t the technology we are using!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi Neo,
Sorry, for now I have no update for it. We're focusing on fixing some
important bugs since xen 3.3.1 is coming...

Thanks,
-- Dexuan


-----Original Message-----
From: Neo Jia [mailto:neojia@gmail.com]
Sent: 2008年9月19日 14:46
To: Cui, Dexuan
Cc: Yuji Shimada; xen-devel@lists.xensource.com
Subject: Re: [Xen-devel] [PATCH] Improve the current FLR logic

Dexuan,

May I know the current development stage of FLR in pciback driver?

Thanks,
Neo

On Thu, Jul 24, 2008 at 3:19 AM, Cui, Dexuan <dexuan.cui@intel.com>
wrote:
> Yuji Shimada wrote:
>>> Yuji Shimada wrote:
>>>> On Wed, 23 Jul 2008 11:16:31 +0800
>>>> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>>>>> I think xenstore seems to be a little ugly place to
remember the
>>>>> proper values of all or part of registers of all the
devices in
>>>>> the system. I agree pciback is a good place.
>>>>> Yes. Now I realize pciback seems to be the best place to do
most of
>>>>> the FLR logic. But this needs modify pciback and add
interfaces for
>>>>> Control Panel. I'm not sure the changes can be small
enought to be
>>>>> in 3.3.
>>>>
>>>> Hi Cui,
>>>>
>>>> What do you think about the following idea?
>>>>   - For 3.3, modify xend to have limited saving/restoring and
>>>> resetting functions.
>>>>   - For 3.4, modify pciback to have proper saving/restoring and
>>>>     resetting functions.
>>> Hi Yuji,
>>> Thanks very much for the suggestions!
>>> I agree.
>>>
>>>> The purpose of saving is to save the values configured by
firmware.
>>>> So the best timing of saving the value is when dom0 starts. But
if
>>>> saving the values when dom0 starts, pciback is the best place
to
>>>> save/restore. If pciback saves/restores the values, we need
many
>>>> codings. So it is difficult to achieve it in 3.3.
>>>>
>>>> So for 3.3, there is a way that saving the value just before
>>>> resetting. It is possible xend saves/restores the value, I
think. In
>>>
>>> In current code of xend,
>>> The steps are:
>>> 1) save the current values of the configuration space registers 2)
>>> do_FLR 3) restore the values of the registers
>>>
>>> "a way that saving the value just before resetting." --
do you mean
>>> the same thing?
>>>
>>
>> Hi Cui,
>>
>> Yes, I mean the same thing. But in current code of xend, xend saves
>> all Configuration Registers. The only registers I suggested before
>> should be saved. Small modification is needed.
>
> It sounds good. Namely, we need to save/restore the following registers
> for now:
>
>    - Base Address Registers
>    - Cache-line size Register
>    - Latency timer Register
>    - Enable SERR Bit/Enable PERR Bit in Device Control Register
>    - Uncorrectable Error Mask Register
>    - Uncorrectable Error Severity Register
>    - Correctable Error Mask Register
>    - Advanced Error Capabilities and Control Register
>    - Device Control Register
>    - Link Control Register
>    - Secondary Uncorrectable Error Severity Register
>    - Secondary Uncorrectable Error Mask Register
>    - Device Control 2 Register
>    - Link Control 2 Register
>    - The following resister should be configured to "0".
>          - PME Enable Bit/PME Status Bit in PCI Power Management
>            Control/Status Register
>
> However, I think maybe the modification is not small enough because
> 1) we need to save each registers one by one using Python script in
> xend, and later restore each registers respectively one by one;
> 2) we should handle bridge in some cases, so we need to distinguish
> bridges from regular devices since the register layouts are different;
> 3) Some of the registers you listed are inside the extended PCIe space,
> so we need detect if a device/bride has the PCIe capability? And find
> each capability/save the register;
> 4) xend uses the sys filesystem to access the registers. For the case of
> PCIe registers, when Dom0 is configured with/without PCIe support (by
> default, it's "without" now), we should detect and treat it
differently?
>
> Acutually looks the save/restore-all-the-256-byte idea (which was in
> hypervisor and is in xend now) works very well for quite a long time,
> and no actual issue is reported as far as I know. Since it looks very
> difficult to do things perfectly for now and we'll improve them by
> changing pciback in the long run, maybe keeping the current simple
> method is acceptable? :-)
>
> Thanks,
> -- Dexuan
>>
>>>> this case, xend don't need saving the values in xenstore.
Because
>>>> interval between saving and restoring is small.
>>>>
>>>> My idea is summarized as follows:
>>>>
>>>> Xen    Timing              Where      Difficulty
>>>> 3.3    before resetting    xend       easy
>>>> 3.4    when Dom0 starts    pciback    hard
>>>>
>>> I exactly agree with you.
>>>
>>
>> Thanks.
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Yuji,

Could you show me how to use the hotplug method to restore the
register? Also, in the current implementation, we just read 4 bytes
each time and write them from 0 to 256 - 4. Will this cause any side
effect?

Thanks,
Neo

On Thu, Jul 17, 2008 at 2:33 AM, Yuji Shimada
<shimada-yxb@necst.nec.co.jp> wrote:
> On Sat, 12 Jul 2008 20:37:34 +0800
> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>
>> I''d like to ask for your comments, and test feedbacks. Thank
you very
>> much!
>
> There is a problem with the device reconfiguring logic. xend saves all
> Configuration Register''s values before reset. And xend writes the
> values to the registers after reset. This means the resister''s
values
> which are configured by guest software are restore. Guest software
> setting should be cleared.
>
> I think the following Configuration Resister should be reconfigured to
> correct values after reset. And other registers should not be
> reconfigured after reset if there is no reason.
>
> - It is necessary to write the base address of the resource allocated
>  by the dom0 kernel to the following resister.
>    - Base Address Register
>
> - When dom0 starts, the values configured by firmware should be saved,
>  and the values should be written to the following resisters after
>  reset. The reason is that firmware configures them to archive system
>  specific functions. Especially, the registers relating error reporting
>  are configured to collect error information. If this configured values
>  is changed, some functions might be lost.
>  Instead of save/restore, it is good way to write the value from _HPX
>  method to registers, I think. _HPX method returns the value to write
>  to registers.
>    - Cache-line size Register
>    - Latency timer Register
>    - Enable SERR Bit/Enable PERR Bit in Device Control Register
>    - Uncorrectable Error Mask Register
>    - Uncorrectable Error Severity Register
>    - Correctable Error Mask Register
>    - Advanced Error Capabilities and Control Register
>    - Device Control Register
>    - Link Control Register
>    - Secondary Uncorrectable Error Severity Register
>    - Secondary Uncorrectable Error Mask Register
>    - Device Control 2 Register
>    - Link Control 2 Register
>
> - The following resister should be configured to "0".
>    - PME Enable Bit/PME Status Bit in PCI Power Management
>      Control/Status Register
>
> I would like to discuss this logic more.
>
> Thanks.
>
> --
> Yuji Shimada
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>


-- 
I would remember that if researchers were not ambitious
probably today we haven''t the technology we are using!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Dexuan,

Could you give me some pointers about the right implementation of
FLR/PCIe reset on pciback driver? I am looking into it now and would
like to get it done.

Thanks,
Neo

On Thu, Sep 18, 2008 at 11:45 PM, Neo Jia <neojia@gmail.com>
wrote:
> Dexuan,
>
> May I know the current development stage of FLR in pciback driver?
>
> Thanks,
> Neo
>
> On Thu, Jul 24, 2008 at 3:19 AM, Cui, Dexuan <dexuan.cui@intel.com>
wrote:
>> Yuji Shimada wrote:
>>>> Yuji Shimada wrote:
>>>>> On Wed, 23 Jul 2008 11:16:31 +0800
>>>>> "Cui, Dexuan" <dexuan.cui@intel.com> wrote:
>>>>>> I think xenstore seems to be a little ugly place to
remember the
>>>>>> proper values of all or part of registers of all the
devices in
>>>>>> the system. I agree pciback is a good place.
>>>>>> Yes. Now I realize pciback seems to be the best place
to do most of
>>>>>> the FLR logic. But this needs modify pciback and add
interfaces for
>>>>>> Control Panel. I''m not sure the changes can be
small enought to be
>>>>>> in 3.3.
>>>>>
>>>>> Hi Cui,
>>>>>
>>>>> What do you think about the following idea?
>>>>>   - For 3.3, modify xend to have limited saving/restoring
and
>>>>> resetting functions.
>>>>>   - For 3.4, modify pciback to have proper saving/restoring
and
>>>>>     resetting functions.
>>>> Hi Yuji,
>>>> Thanks very much for the suggestions!
>>>> I agree.
>>>>
>>>>> The purpose of saving is to save the values configured by
firmware.
>>>>> So the best timing of saving the value is when dom0 starts.
But if
>>>>> saving the values when dom0 starts, pciback is the best
place to
>>>>> save/restore. If pciback saves/restores the values, we need
many
>>>>> codings. So it is difficult to achieve it in 3.3.
>>>>>
>>>>> So for 3.3, there is a way that saving the value just
before
>>>>> resetting. It is possible xend saves/restores the value, I
think. In
>>>>
>>>> In current code of xend,
>>>> The steps are:
>>>> 1) save the current values of the configuration space registers
2)
>>>> do_FLR 3) restore the values of the registers
>>>>
>>>> "a way that saving the value just before resetting."
-- do you mean
>>>> the same thing?
>>>>
>>>
>>> Hi Cui,
>>>
>>> Yes, I mean the same thing. But in current code of xend, xend saves
>>> all Configuration Registers. The only registers I suggested before
>>> should be saved. Small modification is needed.
>>
>> It sounds good. Namely, we need to save/restore the following registers
>> for now:
>>
>>    - Base Address Registers
>>    - Cache-line size Register
>>    - Latency timer Register
>>    - Enable SERR Bit/Enable PERR Bit in Device Control Register
>>    - Uncorrectable Error Mask Register
>>    - Uncorrectable Error Severity Register
>>    - Correctable Error Mask Register
>>    - Advanced Error Capabilities and Control Register
>>    - Device Control Register
>>    - Link Control Register
>>    - Secondary Uncorrectable Error Severity Register
>>    - Secondary Uncorrectable Error Mask Register
>>    - Device Control 2 Register
>>    - Link Control 2 Register
>>    - The following resister should be configured to "0".
>>          - PME Enable Bit/PME Status Bit in PCI Power Management
>>            Control/Status Register
>>
>> However, I think maybe the modification is not small enough because
>> 1) we need to save each registers one by one using Python script in
>> xend, and later restore each registers respectively one by one;
>> 2) we should handle bridge in some cases, so we need to distinguish
>> bridges from regular devices since the register layouts are different;
>> 3) Some of the registers you listed are inside the extended PCIe space,
>> so we need detect if a device/bride has the PCIe capability? And find
>> each capability/save the register;
>> 4) xend uses the sys filesystem to access the registers. For the case
of
>> PCIe registers, when Dom0 is configured with/without PCIe support (by
>> default, it''s "without" now), we should detect and
treat it differently?
>>
>> Acutually looks the save/restore-all-the-256-byte idea (which was in
>> hypervisor and is in xend now) works very well for quite a long time,
>> and no actual issue is reported as far as I know. Since it looks very
>> difficult to do things perfectly for now and we''ll improve
them by
>> changing pciback in the long run, maybe keeping the current simple
>> method is acceptable? :-)
>>
>> Thanks,
>> -- Dexuan
>>>
>>>>> this case, xend don''t need saving the values in
xenstore. Because
>>>>> interval between saving and restoring is small.
>>>>>
>>>>> My idea is summarized as follows:
>>>>>
>>>>> Xen    Timing              Where      Difficulty
>>>>> 3.3    before resetting    xend       easy
>>>>> 3.4    when Dom0 starts    pciback    hard
>>>>>
>>>> I exactly agree with you.
>>>>
>>>
>>> Thanks.
>>
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>>
>
>
>
> --
> I would remember that if researchers were not ambitious
> probably today we haven''t the technology we are using!
>


-- 
I would remember that if researchers were not ambitious
probably today we haven''t the technology we are using!

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, Neo,

On Tue, 30 Sep 2008 23:53:22 -0700
"Neo Jia" <neojia@gmail.com> wrote:
> Yuji,
> 
> Could you show me how to use the hotplug method to restore the
> register?
Following code will help you to know how to use the hotplug
method(_HPP/_HPX method).

linux-2.6.18-xen.hg/drivers/pci/hotplug/pciehp_pci.c:program_fw_provided_values

Note: using the hotplug method is alternative approach to reconfigure
device after reset, without saving/restoring.

> Also, in the current implementation, we just read 4 bytes
> each time and write them from 0 to 256 - 4. Will this cause any side
> effect?
I''m not sure this cause any side effect.

But there is a case where memory enable bit is set to 1, while base
address registers are not programmed. Some devices might be confused.


Rich saving/restoring logic or using the hotplug method are needed for
following reasons.

- Most of the registers should not be restored, because there is a case
  where guest software programs configuration registers to invalid value.

- We need to program FW provided value to some registers to use
  machine''s RAS function. For example, if system support SERR, SERR
  enable bit should be set to 1, so that device reports error.

Thanks,
--
Yuji Shimada


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel