Re: About hotplug multifunction

2011-11-23 Thread Amos Kong

On 09/09/11 15:08, Amos Kong wrote:

Hello all,

I'm working on hotplug pci multifunction.

1. qemu cmdline:
./x86_64-softmmu/qemu-system-x86_64 -snapshot -m 2000 
/home/kvm_autotest_root/images/rhel61-64-virtio.qcow2 -vnc :0 -monitor 
unix:/tmp/a,server,nowait --enable-kvm -net none

2. script to add virtio-blk devices:
for i in `seq 1 7` 0;do
qemu-img create /tmp/resize$i.qcow2 1G -f qcow2
echo drive_add 0x6.$i id=drv$i,if=none,file=/tmp/resize$i.qcow2 | nc -U /tmp/a
echo device_add 
virtio-blk-pci,id=dev$i,drive=drv$i,addr=0x6.$i,multifunction=on | nc -U /tmp/a
done

3. script to add virio-nic devices:
for i in `seq 1 7` 0;do
echo netdev_add tap,id=drv$i | nc -U /tmp/a
echo device_add 
virtio-net-pci,id=dev$i,netdev=drv$i,addr=0x6.$i,multifunction=on | nc -U /tmp/a
done

4. current qemu behaviors
4.1. add func 1~7 one by one, then add func 0
virtio-nic : success, all funcs are added
virtio-blk : success

4.2. add func 0~7 one by one
virtio-nic : failed, only func 0 is added
virtio-blk : success

4.3. removing any single func in monitor
virtio-nic: func 0 are not found in 'lspci', func 1~7 also exist. eth1~eth7 
also exist.
virtio-blk: func 0 are not found in 'lspci', func 1~7 also exist. the device. 
/dev/vda disappears,
   vdb,vdc,vde,vdf,vdg,vdh,vdi,vdj also exist. If I re-add 8 funcs 
to guest, they all works.
   # lspci (00:06.1 ~ 00:06.7 exist, 00:06.0 doesn't exit)
   00:06.1 SCSI storage controller: Red Hat, Inc Virtio block 
device (rev ff)

qemu sends an acpi event to guest, then guest will remove all funcs in the slot.
linux-2.6/drivers/pci/hotplug/acpiphp_glue.c:
static int disable_device(struct acpiphp_slot *slot) {
 list_for_each_entry(func,&slot->funcs, sibling) {
 ...


Hello all,

I re-tested the multifunction hotplug of winxp,rhel6,win7 guest. 
(upstream qemu-kvm)


For all the guests, func0 needs to be added, otherwise no func can be 
found in guest.
RHEL6 needs to add fun0 in the end, otherwise, the funcs added after 
hot-adding func0 could not be found in guest.

For winxp,win7, add func0 in any time, all the funcs can be found in guest.

This is the test result( hogplug order of funcs & funcs can be found in 
guest):

hotplug order   xp  win7rhel6
--  -
func 0~70~7 0~7 0~7
func 0~50~6 0~6 0~6
func 1~4,0,50~5 0~5 0~4
func 1~4no  no  no

It looks like the 'problem' of linux guest, or could we accept to add 
func0 after adding other funcs for linux guest?


Thanks,
Amos


Questions:
1. why func1~7 still can be found after hot-remove? is it same as real hardware?
2. why the func 1~7 could not be added to guest (addingfunc 0~7 one by one)?
3. how about this interface to hotplug/hot-unplug multifunction:
1) Add func 1-7 by monitor, add func 0, then send an acpi event to notice 
guest
2) Remove func0, send an acpi event to guest. (all funcs can be removed)
4. what does "reversion 0xff" stand for?

Thanks in advance,
Amos


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-13 Thread Amos Kong
- Original Message -
> Hi all,

I've tested with WinXp guest, the multifunction hotplug works.

> After reading the pci driver code, I found a problem.
> 
> There is a list for each slot, (slot->funcs)
> it will be inited in acpiphp_glue.c:register_slot() before hotpluging
> device,
> and only one entry(func 0) will be added to it,
> no new entry will be added to the list when hotpluging devices to the
> slot.
> 
> When we release the device, there are only _one_ entry in the
> list(slot->funcs).

This list (slot->funcs) is designed to restore the func entries,
But it only restore the func 0.

I changed the # to # in seabios: src/acpi-dsdt.dsl
mf hotplug of Windows doesn't work. linux guest will only remove the last func, 
func 0~6 still exist in guest.
it seems a bug of Linux pci driver (not calling pci_remove_bus_device() for 
func 1~7).

--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -130,7 +130,7 @@ DefinitionBlock (
 
 #define hotplug_slot(name, nr) \
 Device (S##name) {\
-   Name (_ADR, nr##)  \
+   Name (_ADR, nr##)  \
Method (_EJ0,1) {  \
 Store(ShiftLeft(1, nr), B0EJ) \
 Return (0x0)  \
@@ -462,7 +462,7 @@ DefinitionBlock (
 
 #define gen_pci_device(name, nr)\
 Device(SL##name) {  \
-Name (_ADR, nr##)   \
+Name (_ADR, nr##)   \
 Method (_RMV) { \

==
I try to add new entries in acpiphp_glue.c:enable_device() for each func, but 
it doesn't work.



> acpiphp_glue.c:disable_device()
> list_for_each_entry(func, &slot->funcs, sibling) {
> pdev = pci_get_slot(slot->bridge->pci_bus,
> PCI_DEVFN(slot->device, func->function));
> ...release code... // those code can only be executed one time (func
> 0)
> pci_remove_bus_device(pdev);
> }
> 
> bus.c:pci_bus_add_device() is called for each func device in
> acpiphp_glue.c:enable_device().
> bus.c:pci_remove_bus_device(pdev) is only called for func 0 in
> acpiphp_glue.c:disable_device().
> 
> 
> Resolution: (I've tested it, success)
> enumerate all the funcs when disable device.
> 
> list_for_each_entry(func, &slot->funcs, sibling) {
> for (i=0; i<8; i++) {
> pdev = pci_get_slot(slot->bridge->pci_bus,
> PCI_DEVFN(slot->device, i));
> ...release code...
> pci_remove_bus_device(pdev);
> 
> }
> }


--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-13 Thread Amos Kong
Hi all,

After reading the pci driver code, I found a problem.

There is a list for each slot, (slot->funcs)
it will be inited in acpiphp_glue.c:register_slot() before hotpluging device,
and only one entry(func 0) will be added to it,
no new entry will be added to the list when hotpluging devices to the slot.

When we release the device, there are only _one_ entry in the list(slot->funcs).

acpiphp_glue.c:disable_device()
list_for_each_entry(func, &slot->funcs, sibling) {
pdev = pci_get_slot(slot->bridge->pci_bus,
PCI_DEVFN(slot->device, func->function));
...release code...  // those code can only be executed one time (func 0)
pci_remove_bus_device(pdev);
}

bus.c:pci_bus_add_device() is called for each func device in 
acpiphp_glue.c:enable_device().
bus.c:pci_remove_bus_device(pdev) is only called for func 0 in 
acpiphp_glue.c:disable_device().


Resolution: (I've tested it, success)
enumerate all the funcs when disable device.

list_for_each_entry(func, &slot->funcs, sibling) {
for (i=0; i<8; i++) {
pdev = pci_get_slot(slot->bridge->pci_bus,
PCI_DEVFN(slot->device, i));
...release code...
pci_remove_bus_device(pdev);

}
}

Thanks,
Amos
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] About hotplug multifunction

2011-09-13 Thread Gleb Natapov
On Tue, Sep 13, 2011 at 01:05:00PM +0300, Michael S. Tsirkin wrote:
> On Tue, Sep 13, 2011 at 09:52:49AM +0300, Gleb Natapov wrote:
> > On Tue, Sep 13, 2011 at 02:57:20PM +0900, Isaku Yamahata wrote:
> > > On Sun, Sep 11, 2011 at 12:05:17PM +0300, Michael S. Tsirkin wrote:
> > > > On Sat, Sep 10, 2011 at 02:43:11AM +0900, Isaku Yamahata wrote:
> > > > > pci/pcie hot plug needs clean up for multifunction hotplug in long 
> > > > > term.
> > > > > Only single function device case works. Multifunction case is broken 
> > > > > somwehat.
> > > > > Especially the current acpi based hotplug should be replaced by
> > > > > the standardized hot plug controller in long term.
> > > > 
> > > > We'll need to keep supporting windows XP, which IIUC only
> > > > supports hotplug through ACPI. So it looks like we'll
> > > > need both.
> > > 
> > > Yes, we'll need both then.
> > > It would be possible to implement acpi-based hotplug with
> > > standardized hotplug controller. Not with qemu-specific controller.
> > > 
> > Where is this "standardized hotplug controller" documented?
> 
> Sorry both pci bridge and hotplug spec only reference shpc.
> The spec itself is PCI Standard Hot-Plug
> Controller and Subsystem Specification.
> 
> Revision 1.0 - get it from pcisig
> 
Thanks, Isaku is already pointed it to me.

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] About hotplug multifunction

2011-09-13 Thread Michael S. Tsirkin
On Tue, Sep 13, 2011 at 09:52:49AM +0300, Gleb Natapov wrote:
> On Tue, Sep 13, 2011 at 02:57:20PM +0900, Isaku Yamahata wrote:
> > On Sun, Sep 11, 2011 at 12:05:17PM +0300, Michael S. Tsirkin wrote:
> > > On Sat, Sep 10, 2011 at 02:43:11AM +0900, Isaku Yamahata wrote:
> > > > pci/pcie hot plug needs clean up for multifunction hotplug in long term.
> > > > Only single function device case works. Multifunction case is broken 
> > > > somwehat.
> > > > Especially the current acpi based hotplug should be replaced by
> > > > the standardized hot plug controller in long term.
> > > 
> > > We'll need to keep supporting windows XP, which IIUC only
> > > supports hotplug through ACPI. So it looks like we'll
> > > need both.
> > 
> > Yes, we'll need both then.
> > It would be possible to implement acpi-based hotplug with
> > standardized hotplug controller. Not with qemu-specific controller.
> > 
> Where is this "standardized hotplug controller" documented?

Sorry both pci bridge and hotplug spec only reference shpc.
The spec itself is PCI Standard Hot-Plug
Controller and Subsystem Specification.

Revision 1.0 - get it from pcisig

> > It would require a bit amount of work to write ACPI code in DSDT that
> > handles standardized hotplug controller.
> > So I'm not sure it's worth while only for windows XP support.
> > -- 
> > yamahata
> 
> --
>   Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] About hotplug multifunction

2011-09-13 Thread Michael S. Tsirkin
On Tue, Sep 13, 2011 at 09:52:49AM +0300, Gleb Natapov wrote:
> On Tue, Sep 13, 2011 at 02:57:20PM +0900, Isaku Yamahata wrote:
> > On Sun, Sep 11, 2011 at 12:05:17PM +0300, Michael S. Tsirkin wrote:
> > > On Sat, Sep 10, 2011 at 02:43:11AM +0900, Isaku Yamahata wrote:
> > > > pci/pcie hot plug needs clean up for multifunction hotplug in long term.
> > > > Only single function device case works. Multifunction case is broken 
> > > > somwehat.
> > > > Especially the current acpi based hotplug should be replaced by
> > > > the standardized hot plug controller in long term.
> > > 
> > > We'll need to keep supporting windows XP, which IIUC only
> > > supports hotplug through ACPI. So it looks like we'll
> > > need both.
> > 
> > Yes, we'll need both then.
> > It would be possible to implement acpi-based hotplug with
> > standardized hotplug controller. Not with qemu-specific controller.
> > 
> Where is this "standardized hotplug controller" documented?

In the pci bridge spec.

> > It would require a bit amount of work to write ACPI code in DSDT that
> > handles standardized hotplug controller.
> > So I'm not sure it's worth while only for windows XP support.
> > -- 
> > yamahata
> 
> --
>   Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] About hotplug multifunction

2011-09-13 Thread Isaku Yamahata
On Tue, Sep 13, 2011 at 09:52:49AM +0300, Gleb Natapov wrote:
> On Tue, Sep 13, 2011 at 02:57:20PM +0900, Isaku Yamahata wrote:
> > On Sun, Sep 11, 2011 at 12:05:17PM +0300, Michael S. Tsirkin wrote:
> > > On Sat, Sep 10, 2011 at 02:43:11AM +0900, Isaku Yamahata wrote:
> > > > pci/pcie hot plug needs clean up for multifunction hotplug in long term.
> > > > Only single function device case works. Multifunction case is broken 
> > > > somwehat.
> > > > Especially the current acpi based hotplug should be replaced by
> > > > the standardized hot plug controller in long term.
> > > 
> > > We'll need to keep supporting windows XP, which IIUC only
> > > supports hotplug through ACPI. So it looks like we'll
> > > need both.
> > 
> > Yes, we'll need both then.
> > It would be possible to implement acpi-based hotplug with
> > standardized hotplug controller. Not with qemu-specific controller.
> > 
> Where is this "standardized hotplug controller" documented?

PCI Hot-Plug 1.1 by PCI sig.

NOTE: I already implemented pcie native hotplug in qemu which is defined
in pcie spec. 

> > It would require a bit amount of work to write ACPI code in DSDT that
> > handles standardized hotplug controller.
> > So I'm not sure it's worth while only for windows XP support.
> > -- 
> > yamahata
> 
> --
>   Gleb.
> 

-- 
yamahata
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Qemu-devel] About hotplug multifunction

2011-09-12 Thread Gleb Natapov
On Tue, Sep 13, 2011 at 02:57:20PM +0900, Isaku Yamahata wrote:
> On Sun, Sep 11, 2011 at 12:05:17PM +0300, Michael S. Tsirkin wrote:
> > On Sat, Sep 10, 2011 at 02:43:11AM +0900, Isaku Yamahata wrote:
> > > pci/pcie hot plug needs clean up for multifunction hotplug in long term.
> > > Only single function device case works. Multifunction case is broken 
> > > somwehat.
> > > Especially the current acpi based hotplug should be replaced by
> > > the standardized hot plug controller in long term.
> > 
> > We'll need to keep supporting windows XP, which IIUC only
> > supports hotplug through ACPI. So it looks like we'll
> > need both.
> 
> Yes, we'll need both then.
> It would be possible to implement acpi-based hotplug with
> standardized hotplug controller. Not with qemu-specific controller.
> 
Where is this "standardized hotplug controller" documented?

> It would require a bit amount of work to write ACPI code in DSDT that
> handles standardized hotplug controller.
> So I'm not sure it's worth while only for windows XP support.
> -- 
> yamahata

--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-12 Thread Isaku Yamahata
On Sun, Sep 11, 2011 at 12:05:17PM +0300, Michael S. Tsirkin wrote:
> On Sat, Sep 10, 2011 at 02:43:11AM +0900, Isaku Yamahata wrote:
> > pci/pcie hot plug needs clean up for multifunction hotplug in long term.
> > Only single function device case works. Multifunction case is broken 
> > somwehat.
> > Especially the current acpi based hotplug should be replaced by
> > the standardized hot plug controller in long term.
> 
> We'll need to keep supporting windows XP, which IIUC only
> supports hotplug through ACPI. So it looks like we'll
> need both.

Yes, we'll need both then.
It would be possible to implement acpi-based hotplug with
standardized hotplug controller. Not with qemu-specific controller.

It would require a bit amount of work to write ACPI code in DSDT that
handles standardized hotplug controller.
So I'm not sure it's worth while only for windows XP support.
-- 
yamahata
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-12 Thread Michael S. Tsirkin
On Mon, Sep 12, 2011 at 07:56:07AM -0300, Marcelo Tosatti wrote:
> On Mon, Sep 12, 2011 at 07:21:48AM -0300, Marcelo Tosatti wrote:
> > > We could, for example, keep a stub function 0 around.
> > 
> > I suppose the guest will remove all functions of a device once you
> > attempt to hot-unplug a function.
> > 
> > What is the problem with adding more PCI buses, instead of multifunction
> > ?
> 
> The advantage is that its not only possible to use virtio devices, but any
> kind of PCI device.

Yes. But it is a new feature. Creating multifunction devs is
*already* possible. Hotplug is partially broken,
so getting it into a consistent non buggy state has
value imho.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-12 Thread Marcelo Tosatti
On Mon, Sep 12, 2011 at 07:21:48AM -0300, Marcelo Tosatti wrote:
> > We could, for example, keep a stub function 0 around.
> 
> I suppose the guest will remove all functions of a device once you
> attempt to hot-unplug a function.
> 
> What is the problem with adding more PCI buses, instead of multifunction
> ?

The advantage is that its not only possible to use virtio devices, but any
kind of PCI device.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-12 Thread Marcelo Tosatti
On Sun, Sep 11, 2011 at 09:51:06PM +0300, Michael S. Tsirkin wrote:
> On Sun, Sep 11, 2011 at 12:01:49PM -0300, Marcelo Tosatti wrote:
> > On Sun, Sep 11, 2011 at 12:23:57PM +0300, Michael S. Tsirkin wrote:
> > > On Fri, Sep 09, 2011 at 03:34:26PM -0300, Marcelo Tosatti wrote:
> > > > > > something I noted when readin our acpi code:
> > > > > > we currently pass eject request for function 0 only:
> > > > > >Name (_ADR, nr##)
> > > > > > We either need a device per function there (acpi 1.0),
> > > > > > send eject request for them all, or use 
> > > > > > as function number (newer acpi, not sure which version).
> > > > > > Need to see which guests (windows,linux) can handle which form.
> > > > > 
> > > > > I'd guess we need to change that to .
> > > > 
> > > > No need, only make sure function 0 is there and all other functions
> > > > should be removed automatically by the guest on eject notification.
> > > 
> > > Hmm, the ACPI spec explicitly says:
> > > 
> > > High word = Device #, Low word = Function #.
> > > (e.g., device 3, function 2 is 0x00030002). To refer
> > > to all the functions on a device #, use a function
> > > number of ).
> > 
> > Right, but this is the _ADR of the device instance in ACPI. 
> > The communication between QEMU and the ACPI DSL code is all 
> > based in slots.
> 
> It's easy to extend that if we like though.
> 
> > > > ACPI PCI hotplug is based on slots, not on functions. It does not
> > > > support addition/removal of individual functions.
> > > 
> > > Interesting. Is this just based on general logic,
> > > reading of the linux driver or the ACPI spec?
> > 
> > Its based on Seabios ACPI DST implementation and its relationship with
> > the QEMU implementation in acpi_piix4.c.
> > 
> > > The ACPI spec itself seems pretty vague. All tables
> > > list devices, where each device has an _ADR entry,
> > > which is built up of PCI device # and function #.
> > 
> > Yes, it is vague. Given the mandate from the PCI spec a device _must
> > contain_ function 0, usage (including hotplug/unplug) of individual
> > functions other than 0 as separate devices is a no-go.
> 
> It doesn't seem to be a big issue.
> We could, for example, keep a stub function 0 around.

I suppose the guest will remove all functions of a device once you
attempt to hot-unplug a function.

What is the problem with adding more PCI buses, instead of multifunction
?

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-11 Thread Michael S. Tsirkin
On Sun, Sep 11, 2011 at 12:01:49PM -0300, Marcelo Tosatti wrote:
> On Sun, Sep 11, 2011 at 12:23:57PM +0300, Michael S. Tsirkin wrote:
> > On Fri, Sep 09, 2011 at 03:34:26PM -0300, Marcelo Tosatti wrote:
> > > > > something I noted when readin our acpi code:
> > > > > we currently pass eject request for function 0 only:
> > > > >Name (_ADR, nr##)
> > > > > We either need a device per function there (acpi 1.0),
> > > > > send eject request for them all, or use 
> > > > > as function number (newer acpi, not sure which version).
> > > > > Need to see which guests (windows,linux) can handle which form.
> > > > 
> > > > I'd guess we need to change that to .
> > > 
> > > No need, only make sure function 0 is there and all other functions
> > > should be removed automatically by the guest on eject notification.
> > 
> > Hmm, the ACPI spec explicitly says:
> > 
> > High word = Device #, Low word = Function #.
> > (e.g., device 3, function 2 is 0x00030002). To refer
> > to all the functions on a device #, use a function
> > number of ).
> 
> Right, but this is the _ADR of the device instance in ACPI. 
> The communication between QEMU and the ACPI DSL code is all 
> based in slots.

It's easy to extend that if we like though.

> > > ACPI PCI hotplug is based on slots, not on functions. It does not
> > > support addition/removal of individual functions.
> > 
> > Interesting. Is this just based on general logic,
> > reading of the linux driver or the ACPI spec?
> 
> Its based on Seabios ACPI DST implementation and its relationship with
> the QEMU implementation in acpi_piix4.c.
> 
> > The ACPI spec itself seems pretty vague. All tables
> > list devices, where each device has an _ADR entry,
> > which is built up of PCI device # and function #.
> 
> Yes, it is vague. Given the mandate from the PCI spec a device _must
> contain_ function 0, usage (including hotplug/unplug) of individual
> functions other than 0 as separate devices is a no-go.

It doesn't seem to be a big issue.
We could, for example, keep a stub function 0 around.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-11 Thread Marcelo Tosatti
On Sun, Sep 11, 2011 at 12:23:57PM +0300, Michael S. Tsirkin wrote:
> On Fri, Sep 09, 2011 at 03:34:26PM -0300, Marcelo Tosatti wrote:
> > > > something I noted when readin our acpi code:
> > > > we currently pass eject request for function 0 only:
> > > >Name (_ADR, nr##)
> > > > We either need a device per function there (acpi 1.0),
> > > > send eject request for them all, or use 
> > > > as function number (newer acpi, not sure which version).
> > > > Need to see which guests (windows,linux) can handle which form.
> > > 
> > > I'd guess we need to change that to .
> > 
> > No need, only make sure function 0 is there and all other functions
> > should be removed automatically by the guest on eject notification.
> 
> Hmm, the ACPI spec explicitly says:
> 
> High word = Device #, Low word = Function #.
> (e.g., device 3, function 2 is 0x00030002). To refer
> to all the functions on a device #, use a function
> number of ).

Right, but this is the _ADR of the device instance in ACPI. 
The communication between QEMU and the ACPI DSL code is all 
based in slots.

> > ACPI PCI hotplug is based on slots, not on functions. It does not
> > support addition/removal of individual functions.
> 
> Interesting. Is this just based on general logic,
> reading of the linux driver or the ACPI spec?

Its based on Seabios ACPI DST implementation and its relationship with
the QEMU implementation in acpi_piix4.c.

> The ACPI spec itself seems pretty vague. All tables
> list devices, where each device has an _ADR entry,
> which is built up of PCI device # and function #.

Yes, it is vague. Given the mandate from the PCI spec a device _must
contain_ function 0, usage (including hotplug/unplug) of individual
functions other than 0 as separate devices is a no-go.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-11 Thread Michael S. Tsirkin
On Fri, Sep 09, 2011 at 03:34:26PM -0300, Marcelo Tosatti wrote:
> > > something I noted when readin our acpi code:
> > > we currently pass eject request for function 0 only:
> > >Name (_ADR, nr##)
> > > We either need a device per function there (acpi 1.0),
> > > send eject request for them all, or use 
> > > as function number (newer acpi, not sure which version).
> > > Need to see which guests (windows,linux) can handle which form.
> > 
> > I'd guess we need to change that to .
> 
> No need, only make sure function 0 is there and all other functions
> should be removed automatically by the guest on eject notification.

Hmm, the ACPI spec explicitly says:

High word = Device #, Low word = Function #.
(e.g., device 3, function 2 is 0x00030002). To refer
to all the functions on a device #, use a function
number of ).


> ACPI PCI hotplug is based on slots, not on functions. It does not
> support addition/removal of individual functions.

Interesting. Is this just based on general logic,
reading of the linux driver or the ACPI spec?

The ACPI spec itself seems pretty vague. All tables
list devices, where each device has an _ADR entry,
which is built up of PCI device # and function #.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-11 Thread Michael S. Tsirkin
On Sat, Sep 10, 2011 at 02:43:11AM +0900, Isaku Yamahata wrote:
> pci/pcie hot plug needs clean up for multifunction hotplug in long term.
> Only single function device case works. Multifunction case is broken somwehat.
> Especially the current acpi based hotplug should be replaced by
> the standardized hot plug controller in long term.

We'll need to keep supporting windows XP, which IIUC only
supports hotplug through ACPI. So it looks like we'll
need both.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-09 Thread Marcelo Tosatti
On Fri, Sep 09, 2011 at 10:05:01AM -0700, Alex Williamson wrote:
> On Fri, 2011-09-09 at 10:32 +0300, Michael S. Tsirkin wrote:
> > On Fri, Sep 09, 2011 at 03:08:21AM -0400, Amos Kong wrote:
> > > Hello all,
> > > 
> > > I'm working on hotplug pci multifunction. 
> > > 
> > > 1. qemu cmdline: 
> > > ./x86_64-softmmu/qemu-system-x86_64 -snapshot -m 2000 
> > > /home/kvm_autotest_root/images/rhel61-64-virtio.qcow2 -vnc :0 -monitor 
> > > unix:/tmp/a,server,nowait --enable-kvm -net none
> > > 
> > > 2. script to add virtio-blk devices:
> > > for i in `seq 1 7` 0;do
> > > qemu-img create /tmp/resize$i.qcow2 1G -f qcow2
> > > echo drive_add 0x6.$i id=drv$i,if=none,file=/tmp/resize$i.qcow2 | nc -U 
> > > /tmp/a
> > > echo device_add 
> > > virtio-blk-pci,id=dev$i,drive=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> > > /tmp/a
> > > done
> 
> I don't think it should work this way, there shouldn't be special
> intrinsic meaning that hotplugging func 0 makes the whole device appear.

Function 0 is mandatory. Thats what the guest (at least the Linux
driver) searches when a notification for the slot is received.

> Perhaps we need a notify= device option so we can add devices without
> notifying the guest, then maybe a different command to cause the pci
> slot notification.
>
> > > 3. script to add virio-nic devices:
> > > for i in `seq 1 7` 0;do
> > > echo netdev_add tap,id=drv$i | nc -U /tmp/a
> > > echo device_add 
> > > virtio-net-pci,id=dev$i,netdev=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> > > /tmp/a
> > > done
> > > 
> > > 4. current qemu behaviors
> > > 4.1. add func 1~7 one by one, then add func 0
> > > virtio-nic : success, all funcs are added
> > > virtio-blk : success
> > > 
> > > 4.2. add func 0~7 one by one
> > > virtio-nic : failed, only func 0 is added
> > > virtio-blk : success
> > > 
> > > 4.3. removing any single func in monitor
> > > virtio-nic: func 0 are not found in 'lspci', func 1~7 also exist. 
> > > eth1~eth7 also exist.
> > > virtio-blk: func 0 are not found in 'lspci', func 1~7 also exist. the 
> > > device. /dev/vda disappears,
> > >   vdb,vdc,vde,vdf,vdg,vdh,vdi,vdj also exist. If I re-add 8 
> > > funcs to guest, they all works.
> > >   # lspci (00:06.1 ~ 00:06.7 exist, 00:06.0 doesn't exit)
> > >   00:06.1 SCSI storage controller: Red Hat, Inc Virtio block 
> > > device (rev ff)
> 
> We shouldn't be able to remove single funcs of a multifunction device,
> imho.
> 
> > something I noted when readin our acpi code:
> > we currently pass eject request for function 0 only:
> >Name (_ADR, nr##)
> > We either need a device per function there (acpi 1.0),
> > send eject request for them all, or use 
> > as function number (newer acpi, not sure which version).
> > Need to see which guests (windows,linux) can handle which form.
> 
> I'd guess we need to change that to .

No need, only make sure function 0 is there and all other functions
should be removed automatically by the guest on eject notification.

> > > 
> > > qemu sends an acpi event to guest, then guest will remove all funcs in 
> > > the slot.
> > > linux-2.6/drivers/pci/hotplug/acpiphp_glue.c:
> > > static int disable_device(struct acpiphp_slot *slot) {
> > > list_for_each_entry(func, &slot->funcs, sibling) {
> > > ...
> > > 
> > > Questions:
> > > 1. why func1~7 still can be found after hot-remove? is it same as real 
> > > hardware?
> 
> I think we want to behave the same as adding and removing a complete
> physical devices, which means all the functions get added and removed
> together.  Probably the only time individual functions disappear on real
> hardware is poking chipset registers to hide and expose sub devices.  It
> may not be necessary to make them atomically [in]accessible, but we
> should treat them as a set.

ACPI PCI hotplug is based on slots, not on functions. It does not
support addition/removal of individual functions.

> > > 2. why the func 1~7 could not be added to guest (addingfunc 0~7 one by 
> > > one)?
> > > 3. how about this interface to hotplug/hot-unplug multifunction:
> > >1) Add func 1-7 by monitor, add func 0, then send an acpi event to 
> > > notice guest
> > >2) Remove func0, send an acpi event to guest. (all funcs can be 
> > > removed)
> 
> I think I'd prefer an explicit interface.  Thanks,
> 
> Alex

Function 0 must be present for the guest to detect the device. I do
not see the problem of specifying (and documenting) that the insert
notification is sent for function 0 only.

An explicit interface is going to break the current scheme where a
single "device_add" command also does the notification.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-09 Thread Isaku Yamahata
pci/pcie hot plug needs clean up for multifunction hotplug in long term.
Only single function device case works. Multifunction case is broken somwehat.
Especially the current acpi based hotplug should be replaced by
the standardized hot plug controller in long term.

On Fri, Sep 09, 2011 at 03:08:21AM -0400, Amos Kong wrote:
> Hello all,
> 
> I'm working on hotplug pci multifunction. 
> 
> 1. qemu cmdline: 
> ./x86_64-softmmu/qemu-system-x86_64 -snapshot -m 2000 
> /home/kvm_autotest_root/images/rhel61-64-virtio.qcow2 -vnc :0 -monitor 
> unix:/tmp/a,server,nowait --enable-kvm -net none
> 
> 2. script to add virtio-blk devices:
> for i in `seq 1 7` 0;do
> qemu-img create /tmp/resize$i.qcow2 1G -f qcow2
> echo drive_add 0x6.$i id=drv$i,if=none,file=/tmp/resize$i.qcow2 | nc -U /tmp/a
> echo device_add 
> virtio-blk-pci,id=dev$i,drive=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> /tmp/a
> done
> 
> 3. script to add virio-nic devices:
> for i in `seq 1 7` 0;do
> echo netdev_add tap,id=drv$i | nc -U /tmp/a
> echo device_add 
> virtio-net-pci,id=dev$i,netdev=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> /tmp/a
> done
> 
> 4. current qemu behaviors
> 4.1. add func 1~7 one by one, then add func 0
> virtio-nic : success, all funcs are added
> virtio-blk : success
> 
> 4.2. add func 0~7 one by one
> virtio-nic : failed, only func 0 is added
> virtio-blk : success
> 
> 4.3. removing any single func in monitor
> virtio-nic: func 0 are not found in 'lspci', func 1~7 also exist. eth1~eth7 
> also exist.
> virtio-blk: func 0 are not found in 'lspci', func 1~7 also exist. the device. 
> /dev/vda disappears,
>   vdb,vdc,vde,vdf,vdg,vdh,vdi,vdj also exist. If I re-add 8 funcs 
> to guest, they all works.
>   # lspci (00:06.1 ~ 00:06.7 exist, 00:06.0 doesn't exit)
>   00:06.1 SCSI storage controller: Red Hat, Inc Virtio block 
> device (rev ff)
> 
> qemu sends an acpi event to guest, then guest will remove all funcs in the 
> slot.
> linux-2.6/drivers/pci/hotplug/acpiphp_glue.c:
> static int disable_device(struct acpiphp_slot *slot) {
> list_for_each_entry(func, &slot->funcs, sibling) {
> ...
> 
> Questions:
> 1. why func1~7 still can be found after hot-remove? is it same as real 
> hardware?
> 2. why the func 1~7 could not be added to guest (addingfunc 0~7 one by one)?
> 3. how about this interface to hotplug/hot-unplug multifunction:
>1) Add func 1-7 by monitor, add func 0, then send an acpi event to notice 
> guest
>2) Remove func0, send an acpi event to guest. (all funcs can be removed)
> 4. what does "reversion 0xff" stand for?
> 
> Thanks in advance,
> Amos
> 

-- 
yamahata
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-09 Thread Alex Williamson
On Fri, 2011-09-09 at 10:32 +0300, Michael S. Tsirkin wrote:
> On Fri, Sep 09, 2011 at 03:08:21AM -0400, Amos Kong wrote:
> > Hello all,
> > 
> > I'm working on hotplug pci multifunction. 
> > 
> > 1. qemu cmdline: 
> > ./x86_64-softmmu/qemu-system-x86_64 -snapshot -m 2000 
> > /home/kvm_autotest_root/images/rhel61-64-virtio.qcow2 -vnc :0 -monitor 
> > unix:/tmp/a,server,nowait --enable-kvm -net none
> > 
> > 2. script to add virtio-blk devices:
> > for i in `seq 1 7` 0;do
> > qemu-img create /tmp/resize$i.qcow2 1G -f qcow2
> > echo drive_add 0x6.$i id=drv$i,if=none,file=/tmp/resize$i.qcow2 | nc -U 
> > /tmp/a
> > echo device_add 
> > virtio-blk-pci,id=dev$i,drive=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> > /tmp/a
> > done

I don't think it should work this way, there shouldn't be special
intrinsic meaning that hotplugging func 0 makes the whole device appear.
Perhaps we need a notify= device option so we can add devices without
notifying the guest, then maybe a different command to cause the pci
slot notification.
 
> > 3. script to add virio-nic devices:
> > for i in `seq 1 7` 0;do
> > echo netdev_add tap,id=drv$i | nc -U /tmp/a
> > echo device_add 
> > virtio-net-pci,id=dev$i,netdev=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> > /tmp/a
> > done
> > 
> > 4. current qemu behaviors
> > 4.1. add func 1~7 one by one, then add func 0
> > virtio-nic : success, all funcs are added
> > virtio-blk : success
> > 
> > 4.2. add func 0~7 one by one
> > virtio-nic : failed, only func 0 is added
> > virtio-blk : success
> > 
> > 4.3. removing any single func in monitor
> > virtio-nic: func 0 are not found in 'lspci', func 1~7 also exist. eth1~eth7 
> > also exist.
> > virtio-blk: func 0 are not found in 'lspci', func 1~7 also exist. the 
> > device. /dev/vda disappears,
> >   vdb,vdc,vde,vdf,vdg,vdh,vdi,vdj also exist. If I re-add 8 
> > funcs to guest, they all works.
> >   # lspci (00:06.1 ~ 00:06.7 exist, 00:06.0 doesn't exit)
> >   00:06.1 SCSI storage controller: Red Hat, Inc Virtio block 
> > device (rev ff)

We shouldn't be able to remove single funcs of a multifunction device,
imho.

> something I noted when readin our acpi code:
> we currently pass eject request for function 0 only:
>Name (_ADR, nr##)
> We either need a device per function there (acpi 1.0),
> send eject request for them all, or use 
> as function number (newer acpi, not sure which version).
> Need to see which guests (windows,linux) can handle which form.

I'd guess we need to change that to .

> > 
> > qemu sends an acpi event to guest, then guest will remove all funcs in the 
> > slot.
> > linux-2.6/drivers/pci/hotplug/acpiphp_glue.c:
> > static int disable_device(struct acpiphp_slot *slot) {
> > list_for_each_entry(func, &slot->funcs, sibling) {
> > ...
> > 
> > Questions:
> > 1. why func1~7 still can be found after hot-remove? is it same as real 
> > hardware?

I think we want to behave the same as adding and removing a complete
physical devices, which means all the functions get added and removed
together.  Probably the only time individual functions disappear on real
hardware is poking chipset registers to hide and expose sub devices.  It
may not be necessary to make them atomically [in]accessible, but we
should treat them as a set.

> > 2. why the func 1~7 could not be added to guest (addingfunc 0~7 one by one)?
> > 3. how about this interface to hotplug/hot-unplug multifunction:
> >1) Add func 1-7 by monitor, add func 0, then send an acpi event to 
> > notice guest
> >2) Remove func0, send an acpi event to guest. (all funcs can be removed)

I think I'd prefer an explicit interface.  Thanks,

Alex

> We must make sure guest acked removal of all functions. Surprise
> removal would be bad.
> 
> > 4. what does "reversion 0xff" stand for?
> 
> You get  if no device responds to a configuration read.
> 



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: About hotplug multifunction

2011-09-09 Thread Michael S. Tsirkin
On Fri, Sep 09, 2011 at 03:08:21AM -0400, Amos Kong wrote:
> Hello all,
> 
> I'm working on hotplug pci multifunction. 
> 
> 1. qemu cmdline: 
> ./x86_64-softmmu/qemu-system-x86_64 -snapshot -m 2000 
> /home/kvm_autotest_root/images/rhel61-64-virtio.qcow2 -vnc :0 -monitor 
> unix:/tmp/a,server,nowait --enable-kvm -net none
> 
> 2. script to add virtio-blk devices:
> for i in `seq 1 7` 0;do
> qemu-img create /tmp/resize$i.qcow2 1G -f qcow2
> echo drive_add 0x6.$i id=drv$i,if=none,file=/tmp/resize$i.qcow2 | nc -U /tmp/a
> echo device_add 
> virtio-blk-pci,id=dev$i,drive=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> /tmp/a
> done
> 
> 3. script to add virio-nic devices:
> for i in `seq 1 7` 0;do
> echo netdev_add tap,id=drv$i | nc -U /tmp/a
> echo device_add 
> virtio-net-pci,id=dev$i,netdev=drv$i,addr=0x6.$i,multifunction=on | nc -U 
> /tmp/a
> done
> 
> 4. current qemu behaviors
> 4.1. add func 1~7 one by one, then add func 0
> virtio-nic : success, all funcs are added
> virtio-blk : success
> 
> 4.2. add func 0~7 one by one
> virtio-nic : failed, only func 0 is added
> virtio-blk : success
> 
> 4.3. removing any single func in monitor
> virtio-nic: func 0 are not found in 'lspci', func 1~7 also exist. eth1~eth7 
> also exist.
> virtio-blk: func 0 are not found in 'lspci', func 1~7 also exist. the device. 
> /dev/vda disappears,
>   vdb,vdc,vde,vdf,vdg,vdh,vdi,vdj also exist. If I re-add 8 funcs 
> to guest, they all works.
>   # lspci (00:06.1 ~ 00:06.7 exist, 00:06.0 doesn't exit)
>   00:06.1 SCSI storage controller: Red Hat, Inc Virtio block 
> device (rev ff)

something I noted when readin our acpi code:
we currently pass eject request for function 0 only:
   Name (_ADR, nr##)
We either need a device per function there (acpi 1.0),
send eject request for them all, or use 
as function number (newer acpi, not sure which version).
Need to see which guests (windows,linux) can handle which form.

> 
> qemu sends an acpi event to guest, then guest will remove all funcs in the 
> slot.
> linux-2.6/drivers/pci/hotplug/acpiphp_glue.c:
> static int disable_device(struct acpiphp_slot *slot) {
> list_for_each_entry(func, &slot->funcs, sibling) {
> ...
> 
> Questions:
> 1. why func1~7 still can be found after hot-remove? is it same as real 
> hardware?
> 2. why the func 1~7 could not be added to guest (addingfunc 0~7 one by one)?
> 3. how about this interface to hotplug/hot-unplug multifunction:
>1) Add func 1-7 by monitor, add func 0, then send an acpi event to notice 
> guest
>2) Remove func0, send an acpi event to guest. (all funcs can be removed)

We must make sure guest acked removal of all functions. Surprise
removal would be bad.

> 4. what does "reversion 0xff" stand for?

You get  if no device responds to a configuration read.

-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


About hotplug multifunction

2011-09-09 Thread Amos Kong
Hello all,

I'm working on hotplug pci multifunction. 

1. qemu cmdline: 
./x86_64-softmmu/qemu-system-x86_64 -snapshot -m 2000 
/home/kvm_autotest_root/images/rhel61-64-virtio.qcow2 -vnc :0 -monitor 
unix:/tmp/a,server,nowait --enable-kvm -net none

2. script to add virtio-blk devices:
for i in `seq 1 7` 0;do
qemu-img create /tmp/resize$i.qcow2 1G -f qcow2
echo drive_add 0x6.$i id=drv$i,if=none,file=/tmp/resize$i.qcow2 | nc -U /tmp/a
echo device_add 
virtio-blk-pci,id=dev$i,drive=drv$i,addr=0x6.$i,multifunction=on | nc -U /tmp/a
done

3. script to add virio-nic devices:
for i in `seq 1 7` 0;do
echo netdev_add tap,id=drv$i | nc -U /tmp/a
echo device_add 
virtio-net-pci,id=dev$i,netdev=drv$i,addr=0x6.$i,multifunction=on | nc -U /tmp/a
done

4. current qemu behaviors
4.1. add func 1~7 one by one, then add func 0
virtio-nic : success, all funcs are added
virtio-blk : success

4.2. add func 0~7 one by one
virtio-nic : failed, only func 0 is added
virtio-blk : success

4.3. removing any single func in monitor
virtio-nic: func 0 are not found in 'lspci', func 1~7 also exist. eth1~eth7 
also exist.
virtio-blk: func 0 are not found in 'lspci', func 1~7 also exist. the device. 
/dev/vda disappears,
  vdb,vdc,vde,vdf,vdg,vdh,vdi,vdj also exist. If I re-add 8 funcs 
to guest, they all works.
  # lspci (00:06.1 ~ 00:06.7 exist, 00:06.0 doesn't exit)
  00:06.1 SCSI storage controller: Red Hat, Inc Virtio block device 
(rev ff)

qemu sends an acpi event to guest, then guest will remove all funcs in the slot.
linux-2.6/drivers/pci/hotplug/acpiphp_glue.c:
static int disable_device(struct acpiphp_slot *slot) {
list_for_each_entry(func, &slot->funcs, sibling) {
...

Questions:
1. why func1~7 still can be found after hot-remove? is it same as real hardware?
2. why the func 1~7 could not be added to guest (addingfunc 0~7 one by one)?
3. how about this interface to hotplug/hot-unplug multifunction:
   1) Add func 1-7 by monitor, add func 0, then send an acpi event to notice 
guest
   2) Remove func0, send an acpi event to guest. (all funcs can be removed)
4. what does "reversion 0xff" stand for?

Thanks in advance,
Amos
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html