Xen devel - Dec 2012 - [PATCH v2] qemu-xen:Correctly expose PCH ISA bridge for IGD passthrough

Fix IGD passthrough logic to properly expose PCH ISA bridge (instead
of exposing as pci-pci bridge). The i915 driver require this to
correctly detect the PCH version and enable version specific code
path.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
Timothy Guo <firemeteor@users.sourceforge.net>

diff --git a/hw/pci.c b/hw/pci.c
index f051de1..d371bd7 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -871,11 +871,6 @@ void pci_unplug_netifs(void)
     }
 }

-typedef struct {
-    PCIDevice dev;
-    PCIBus *bus;
-} PCIBridge;
-
 void pci_bridge_write_config(
PCIDevice *d,
                              uint32_t address, uint32_t val, int len)
 {
diff --git a/hw/pci.h b/hw/pci.h
index edc58b6..c2acab9 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -222,6 +222,11 @@ struct PCIDevice {
     int irq_state[4];
 };

+typedef struct {
+    PCIDevice dev;
+    PCIBus *bus;
+} PCIBridge;
+
 extern char direct_pci_str[];
 extern int direct_pci_msitranslate;
 extern int direct_pci_power_mgmt;
diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
index c6f8869..de21f90 100644
--- a/hw/pt-graphics.c
+++ b/hw/pt-graphics.c
@@ -3,6 +3,7 @@
  */

 #include "pass-through.h"
+#include "pci.h"
 #include "pci/header.h"
 #include "pci/pci.h"

@@ -40,9 +41,26 @@ void intel_pch_init(PCIBus *bus)
     did = pt_pci_host_read(pci_dev_1f, PCI_DEVICE_ID, 2);
     rid = pt_pci_host_read(pci_dev_1f, PCI_REVISION, 1);

-    if ( vid == PCI_VENDOR_ID_INTEL )
-        pci_bridge_init(bus, PCI_DEVFN(0x1f, 0), vid, did, rid,
-                        pch_map_irq, "intel_bridge_1f");
+    if (vid == PCI_VENDOR_ID_INTEL) {
+        PCIBridge *s = (PCIBridge *)pci_register_device(bus,
"intel_bridge_1f",
+                sizeof(PCIBridge), PCI_DEVFN(0x1f, 0), NULL,
pci_bridge_write_config);
+
+        pci_config_set_vendor_id(s->dev.config, vid);
+        pci_config_set_device_id(s->dev.config, did);
+
+        s->dev.config[PCI_COMMAND] = 0x06; // command = bus master, pci mem
+        s->dev.config[PCI_COMMAND + 1] = 0x00;
+        s->dev.config[PCI_STATUS] = 0xa0; // status = fast
back-to-back, 66MHz, no error
+        s->dev.config[PCI_STATUS + 1] = 0x00; // status = fast devsel
+        s->dev.config[PCI_REVISION] = rid;
+        s->dev.config[PCI_CLASS_PROG] = 0x00; // programming i/f
+        pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_ISA);
+        s->dev.config[PCI_LATENCY_TIMER] = 0x10;
+        s->dev.config[PCI_HEADER_TYPE] = 0x80;
+        s->dev.config[PCI_SEC_STATUS] = 0xa0;
+
+        s->bus = pci_register_secondary_bus(&s->dev, pch_map_irq);
+    }
 }

 uint32_t igd_read_opregion(struct pt_dev *pci_dev)

On Wed, 19 Dec 2012, G.R. wrote:
> Fix IGD passthrough logic to properly expose PCH ISA bridge (instead
> of exposing as pci-pci bridge). The i915 driver require this to
> correctly detect the PCH version and enable version specific code
> path.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
> Timothy Guo <firemeteor@users.sourceforge.net>
The patch is fine by me
> diff --git a/hw/pci.c b/hw/pci.c
> index f051de1..d371bd7 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -871,11 +871,6 @@ void pci_unplug_netifs(void)
>      }
>  }
> 
> -typedef struct {
> -    PCIDevice dev;
> -    PCIBus *bus;
> -} PCIBridge;
> -
>  void pci_bridge_write_config(
> PCIDevice *d,
>                               uint32_t address, uint32_t val, int len)
>  {
> diff --git a/hw/pci.h b/hw/pci.h
> index edc58b6..c2acab9 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -222,6 +222,11 @@ struct PCIDevice {
>      int irq_state[4];
>  };
> 
> +typedef struct {
> +    PCIDevice dev;
> +    PCIBus *bus;
> +} PCIBridge;
> +
>  extern char direct_pci_str[];
>  extern int direct_pci_msitranslate;
>  extern int direct_pci_power_mgmt;
> diff --git a/hw/pt-graphics.c b/hw/pt-graphics.c
> index c6f8869..de21f90 100644
> --- a/hw/pt-graphics.c
> +++ b/hw/pt-graphics.c
> @@ -3,6 +3,7 @@
>   */
> 
>  #include "pass-through.h"
> +#include "pci.h"
>  #include "pci/header.h"
>  #include "pci/pci.h"
> 
> @@ -40,9 +41,26 @@ void intel_pch_init(PCIBus *bus)
>      did = pt_pci_host_read(pci_dev_1f, PCI_DEVICE_ID, 2);
>      rid = pt_pci_host_read(pci_dev_1f, PCI_REVISION, 1);
> 
> -    if ( vid == PCI_VENDOR_ID_INTEL )
> -        pci_bridge_init(bus, PCI_DEVFN(0x1f, 0), vid, did, rid,
> -                        pch_map_irq, "intel_bridge_1f");
> +    if (vid == PCI_VENDOR_ID_INTEL) {
> +        PCIBridge *s = (PCIBridge *)pci_register_device(bus,
"intel_bridge_1f",
> +                sizeof(PCIBridge), PCI_DEVFN(0x1f, 0), NULL,
> pci_bridge_write_config);
> +
> +        pci_config_set_vendor_id(s->dev.config, vid);
> +        pci_config_set_device_id(s->dev.config, did);
> +
> +        s->dev.config[PCI_COMMAND] = 0x06; // command = bus master, pci
mem
> +        s->dev.config[PCI_COMMAND + 1] = 0x00;
> +        s->dev.config[PCI_STATUS] = 0xa0; // status = fast
> back-to-back, 66MHz, no error
> +        s->dev.config[PCI_STATUS + 1] = 0x00; // status = fast devsel
> +        s->dev.config[PCI_REVISION] = rid;
> +        s->dev.config[PCI_CLASS_PROG] = 0x00; // programming i/f
> +        pci_config_set_class(s->dev.config, PCI_CLASS_BRIDGE_ISA);
> +        s->dev.config[PCI_LATENCY_TIMER] = 0x10;
> +        s->dev.config[PCI_HEADER_TYPE] = 0x80;
> +        s->dev.config[PCI_SEC_STATUS] = 0xa0;
> +
> +        s->bus = pci_register_secondary_bus(&s->dev,
pch_map_irq);
> +    }
>  }
> 
>  uint32_t igd_read_opregion(struct pt_dev *pci_dev)
>

G.R. writes ("[Xen-devel] [PATCH v2] qemu-xen:Correctly expose PCH ISA
bridge for IGD passthrough"):
> Fix IGD passthrough logic to properly expose PCH ISA bridge (instead
> of exposing as pci-pci bridge). The i915 driver require this to
> correctly detect the PCH version and enable version specific code
> path.
Applied, thanks.  I fixed up some wrap damage.

Ian.

>>> On 22.02.13 at 19:05, Ian Jackson <Ian.Jackson@eu.citrix.com>
wrote:
> G.R. writes ("[Xen-devel] [PATCH v2] qemu-xen:Correctly expose PCH ISA
bridge
> for IGD passthrough"):
>> Fix IGD passthrough logic to properly expose PCH ISA bridge (instead
>> of exposing as pci-pci bridge). The i915 driver require this to
>> correctly detect the PCH version and enable version specific code
>> path.
> 
> Applied, thanks.  I fixed up some wrap damage.
Which means that my pointing out of shortcomings in this patch
got completely ignored - we can only hope that this won''t bite us
later...

Jan

Jan Beulich writes ("Re: [Xen-devel] [PATCH v2] qemu-xen:Correctly expose
PCH ISA bridge for IGD passthrough"):
> > Applied, thanks.  I fixed up some wrap damage.
> 
> Which means that my pointing out of shortcomings in this patch
> got completely ignored - we can only hope that this won''t bite us
> later...
Sorry about that, perhaps I didn''t spot the thread.  Do you think it
should be reverted ?

Ian.

>>> On 25.02.13 at 12:24, Ian Jackson <Ian.Jackson@eu.citrix.com>
wrote:
> Jan Beulich writes ("Re: [Xen-devel] [PATCH v2] qemu-xen:Correctly
expose PCH
> ISA bridge for IGD passthrough"):
>> > Applied, thanks.  I fixed up some wrap damage.
>> 
>> Which means that my pointing out of shortcomings in this patch
>> got completely ignored - we can only hope that this won''t bite
us
>> later...
> 
> Sorry about that, perhaps I didn''t spot the thread.  Do you think
it
> should be reverted ?
I''d prefer if you did so, and wait for a cleaned up version of the
patch to be submitted. If you leave in what''s there right now, I
would b afraid that we''d never get to see a fixup patch on top.

Jan

On Mon, 25 Feb 2013, Jan Beulich wrote:
> >>> On 25.02.13 at 12:24, Ian Jackson
<Ian.Jackson@eu.citrix.com> wrote:
> > Jan Beulich writes ("Re: [Xen-devel] [PATCH v2]
qemu-xen:Correctly expose PCH
> > ISA bridge for IGD passthrough"):
> >> > Applied, thanks.  I fixed up some wrap damage.
> >> 
> >> Which means that my pointing out of shortcomings in this patch
> >> got completely ignored - we can only hope that this won''t
bite us
> >> later...
> > 
> > Sorry about that, perhaps I didn''t spot the thread.  Do you
think it
> > should be reverted ?
> 
> I''d prefer if you did so, and wait for a cleaned up version of the
> patch to be submitted. If you leave in what''s there right now, I
> would b afraid that we''d never get to see a fixup patch on top.
You are talking about 5114F15002000078000BD2B2@nat28.tlf.novell.com?
G.R., can you work on this patch a bit more and address Jan''s comments?
It shouldn''t take long..

Jan Beulich writes ("Re: [Xen-devel] [PATCH v2] qemu-xen:Correctly
expose PCH ISA bridge for IGD passthrough"):
> [Ian Jackson:]
> > Sorry about that, perhaps I didn''t spot the thread.  Do you
think it
> > should be reverted ?
> 
> I''d prefer if you did so, and wait for a cleaned up version of the
> patch to be submitted. If you leave in what''s there right now, I
> would b afraid that we''d never get to see a fixup patch on top.
Done.

Ian.

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH v2] qemu-xen:Correctly
expose PCH ISA bridge for IGD passthrough"):
> On Mon, 25 Feb 2013, Jan Beulich wrote:
> > >>> On 25.02.13 at 12:24, Ian Jackson
<Ian.Jackson@eu.citrix.com> wrote:
> > > Jan Beulich writes ("Re: [Xen-devel] [PATCH v2]
qemu-xen:Correctly expose PCH
> > >> Which means that my pointing out of shortcomings in this
patch
> > >> got completely ignored - we can only hope that this
won''t bite us
> > >> later...
> > > 
> > > Sorry about that, perhaps I didn''t spot the thread.  Do
you think it
> > > should be reverted ?
> > 
> > I''d prefer if you did so, and wait for a cleaned up version
of the
> > patch to be submitted. If you leave in what''s there right
now, I
> > would b afraid that we''d never get to see a fixup patch on
top.
> 
> You are talking about 5114F15002000078000BD2B2@nat28.tlf.novell.com?
> G.R., can you work on this patch a bit more and address Jan''s
comments?
> It shouldn''t take long..
Stefano Stabellini writes ("Re: [Xen-devel] Patch series for IGD
passthrough"):
> On Tue, 16 Apr 2013, George Dunlap wrote:
> > On Mon, Apr 15, 2013 at 9:48 PM, Pasi Kärkkäinen <pasik@iki.fi>
wrote:
> > > On Wed, Mar 20, 2013 at 07:17:14PM +0200, Pasi Kärkkäinen wrote:
> > >> On Fri, Feb 08, 2013 at 12:12:05AM +0800, Rui Guo wrote:
> > >> > This series contains all the fixes required to produce a
working IGD
> > >> > passthrough box. All the changes are previously seen in
the dev list but
> > >> > not yet accepted. Some of them are out-dated and need
some reshape.
> > >> >
> > >> > Detailed description can be found later in each patch.
> > >> >
> > >> > . [PATCH 1/3] qemu-xen-trad/pt_msi_disable: do not clear
all MSI flags
> > >> > . [PATCH 2/3] qemu-xen-trad: Correctly expose PCH ISA
bridge for IGD
> > >> > . [PATCH 3/3] qemu-xen-trad: IGD passthrough: Expose
vendor specific
> > >> >
> > >>
> > >> Looking at qemu-xen-unstable I think patches 2 and 3 are not
yet applied.
> > >> (patch 2 was applied earlier but it got reverted).
> > >>
> > >> Is it likely that we''ll get these merged in for Xen
4.3 ?
Have you addressed Jan''s comments from February ?  I don''t see
anything saying you have done so.

Also please could you CC patches to the maintainer (ie, me, in this
case) and also to people who have commented on them in the past (Jan,
in this case).

Simply constantly pinging is not appropriate.

Ian.

On Tue, May 07, 2013 at 06:12:30PM +0100, Ian Jackson
wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] [PATCH v2]
qemu-xen:Correctly expose PCH ISA bridge for IGD passthrough"):
> > On Mon, 25 Feb 2013, Jan Beulich wrote:
> > > >>> On 25.02.13 at 12:24, Ian Jackson
<Ian.Jackson@eu.citrix.com> wrote:
> > > > Jan Beulich writes ("Re: [Xen-devel] [PATCH v2]
qemu-xen:Correctly expose PCH
> > > >> Which means that my pointing out of shortcomings in this
patch
> > > >> got completely ignored - we can only hope that this
won''t bite us
> > > >> later...
> > > > 
> > > > Sorry about that, perhaps I didn''t spot the thread.
Do you think it
> > > > should be reverted ?
> > > 
> > > I''d prefer if you did so, and wait for a cleaned up
version of the
> > > patch to be submitted. If you leave in what''s there
right now, I
> > > would b afraid that we''d never get to see a fixup patch
on top.
> > 
> > You are talking about 5114F15002000078000BD2B2@nat28.tlf.novell.com?
> > G.R., can you work on this patch a bit more and address Jan''s
comments?
> > It shouldn''t take long..
> 
So this "PATCH v2" here was posted on 19 Dec 2012. It got applied, 
and then reverted due to Jan''s comments/concerns.

"Patch series for IGD passthrough" has a newer version of this patch,
posted on 08 Feb 2013.

> Stefano Stabellini writes ("Re: [Xen-devel] Patch series for IGD
passthrough"):
> > On Tue, 16 Apr 2013, George Dunlap wrote:
> > > On Mon, Apr 15, 2013 at 9:48 PM, Pasi Kärkkäinen
<pasik@iki.fi> wrote:
> > > > On Wed, Mar 20, 2013 at 07:17:14PM +0200, Pasi Kärkkäinen
wrote:
> > > >> On Fri, Feb 08, 2013 at 12:12:05AM +0800, Rui Guo wrote:
> > > >> > This series contains all the fixes required to
produce a working IGD
> > > >> > passthrough box. All the changes are previously
seen in the dev list but
> > > >> > not yet accepted. Some of them are out-dated and
need some reshape.
> > > >> >
> > > >> > Detailed description can be found later in each
patch.
> > > >> >
> > > >> > . [PATCH 1/3] qemu-xen-trad/pt_msi_disable: do not
clear all MSI flags
> > > >> > . [PATCH 2/3] qemu-xen-trad: Correctly expose PCH
ISA bridge for IGD
> > > >> > . [PATCH 3/3] qemu-xen-trad: IGD passthrough:
Expose vendor specific
> > > >> >
> > > >>
> > > >> Looking at qemu-xen-unstable I think patches 2 and 3 are
not yet applied.
> > > >> (patch 2 was applied earlier but it got reverted).
> > > >>
> > > >> Is it likely that we''ll get these merged in for
Xen 4.3 ?
> 
> Have you addressed Jan''s comments from February ?  I
don''t see
> anything saying you have done so.
> 
"Patch series for IGD passthrough" has a later version of the patch in
question.
I''m not sure if Jan''s comments are addressed properly,
there''s no changelog on the patch,
but the patch seems different from the one that got applied+reverted.

> Also please could you CC patches to the maintainer (ie, me, in this
> case) and also to people who have commented on them in the past (Jan,
> in this case).
> 
> Simply constantly pinging is not appropriate.
> 
Sorry for that, but I just wanted to get some attention to these patches,
because it wasn''t clear what still needs to be done to get them merged.

All three patches on the "Patch series for IGD passthrough" are ACKed
by Stefano:
http://lists.xen.org/archives/html/xen-devel/2013-02/msg00593.html


patch 1/3: This one seems to have been merged already to qemu-xen-unstable.
patch 2/3: this one got some comments, but to me it looks like it
didn''t get a NACK? (and it has Stefano''s ACK).
patch 3/3: This one might still have un-addressed comments?


Jan: Can you please re-check 2/3 and 3/3 ? 

"[PATCH 2/3] qemu-xen-trad: Correctly expose PCH ISA bridge for IGD
passthrough":
http://lists.xen.org/archives/html/xen-devel/2013-02/msg00536.html

"[PATCH 3/3] qemu-xen-trad: IGD passthrough: Expose vendor specific pci cap
on host bridge":
http://lists.xen.org/archives/html/xen-devel/2013-02/msg00538.html


Thanks,

-- Pasi

>>> On 09.05.13 at 15:17, Pasi Kärkkäinen<pasik@iki.fi> wrote:
> Jan: Can you please re-check 2/3 and 3/3 ? 
> 
> "[PATCH 2/3] qemu-xen-trad: Correctly expose PCH ISA bridge for IGD 
> passthrough":
> http://lists.xen.org/archives/html/xen-devel/2013-02/msg00536.html 
I'm not sure what you're asking for - my comments there weren't
followed up with by the submitter, so I also don't have anything
new to add. I don't think I've seen a cleaned up patch so far.
> "[PATCH 3/3] qemu-xen-trad: IGD passthrough: Expose vendor specific
pci cap on
> host bridge":
> http://lists.xen.org/archives/html/xen-devel/2013-02/msg00538.html 
Quite similar here, except that Jean was also asked to comment
on why some previously existing code is the way it is, and I don't
think any explanation was ever given.

I can only second what Ian already said - pinging on a patch that
has responses missing from the originator is bad practice. If you
wanted to ping anyone, you'd have to ping those who failed to
answer questions raised to them, not give the appearance of
requesting a patch to be taken that is only half baked.

Jan

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

On Fri, May 10, 2013 at 10:12:31AM +0100, Jan Beulich
wrote:
> >>> On 09.05.13 at 15:17, Pasi Kärkkäinen<pasik@iki.fi>
wrote:
> > Jan: Can you please re-check 2/3 and 3/3 ? 
> > 
> > "[PATCH 2/3] qemu-xen-trad: Correctly expose PCH ISA bridge for
IGD
> > passthrough":
> > http://lists.xen.org/archives/html/xen-devel/2013-02/msg00536.html 
> 
> I''m not sure what you''re asking for - my comments there
weren''t
> followed up with by the submitter, so I also don''t have anything
> new to add. I don''t think I''ve seen a cleaned up patch so
far.
> 
Ok. Thanks for clarifying that (I wasn''t sure if there was still
unresolved issues or not).

> > "[PATCH 3/3] qemu-xen-trad: IGD passthrough: Expose vendor
specific pci cap on
> > host bridge":
> > http://lists.xen.org/archives/html/xen-devel/2013-02/msg00538.html 
> 
> Quite similar here, except that Jean was also asked to comment
> on why some previously existing code is the way it is, and I don''t
> think any explanation was ever given.
>
Ok. 
 
> I can only second what Ian already said - pinging on a patch that
> has responses missing from the originator is bad practice. If you
> wanted to ping anyone, you''d have to ping those who failed to
> answer questions raised to them, not give the appearance of
> requesting a patch to be taken that is only half baked.
> 
I mostly wanted to clarify the situation with these patches,
I''m not suggesting to merge half baked patches. 

So thanks for the comments!

-- Pasi

On Fri, 2013-05-10 at 10:40 +0100, Pasi Kärkkäinen
wrote:
> > I can only second what Ian already said - pinging on a patch that
> > has responses missing from the originator is bad practice. If you
> > wanted to ping anyone, you'd have to ping those who failed to
> > answer questions raised to them, not give the appearance of
> > requesting a patch to be taken that is only half baked.
> > 
> 
> I mostly wanted to clarify the situation with these patches,
Instead of adding more work to our already overburdened maintainers
please start in these cases by working with the submitter.

It is the submitters responsibility to respond to queries and to keep
pushing the patch as necessary. If the submitter isn't engaged any more
then you need to either pick it up yourself or find someone else who is
willing to carry the patch forward, there's no point asking the
maintainers to clarify the situation if no one is around who is going to
act on their responses.

Ian.


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

On 10/05/13 11:00, Ian Campbell wrote:
> On Fri, 2013-05-10 at 10:40 +0100, Pasi Kärkkäinen wrote:
>>> I can only second what Ian already said - pinging on a patch that
>>> has responses missing from the originator is bad practice. If you
>>> wanted to ping anyone, you'd have to ping those who failed to
>>> answer questions raised to them, not give the appearance of
>>> requesting a patch to be taken that is only half baked.
>>>
>> I mostly wanted to clarify the situation with these patches,
> Instead of adding more work to our already overburdened maintainers
> please start in these cases by working with the submitter.
>
> It is the submitters responsibility to respond to queries and to keep
> pushing the patch as necessary. If the submitter isn't engaged any more
> then you need to either pick it up yourself or find someone else who is
> willing to carry the patch forward, there's no point asking the
> maintainers to clarify the situation if no one is around who is going to
> act on their responses.
But if he were going to pick it up, or if he wants to go work with the 
submitter, who may not have a clear understanding what the delay was 
about or how the development process works, then he'd first need to 
figure out what the status was, right?

This sort of "facilitating" role seems like a useful thing to do to 
allow contributions which would otherwise flounder to make it above the 
bar into acceptance.

  -George

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

On Fri, 2013-05-10 at 11:09 +0100, George Dunlap wrote:
> On 10/05/13 11:00, Ian Campbell wrote:
> > On Fri, 2013-05-10 at 10:40 +0100, Pasi Kärkkäinen wrote:
> >>> I can only second what Ian already said - pinging on a patch
that
> >>> has responses missing from the originator is bad practice. If
you
> >>> wanted to ping anyone, you'd have to ping those who failed
to
> >>> answer questions raised to them, not give the appearance of
> >>> requesting a patch to be taken that is only half baked.
> >>>
> >> I mostly wanted to clarify the situation with these patches,
> > Instead of adding more work to our already overburdened maintainers
> > please start in these cases by working with the submitter.
> >
> > It is the submitters responsibility to respond to queries and to keep
> > pushing the patch as necessary. If the submitter isn't engaged any
more
> > then you need to either pick it up yourself or find someone else who
is
> > willing to carry the patch forward, there's no point asking the
> > maintainers to clarify the situation if no one is around who is going
to
> > act on their responses.
> 
> But if he were going to pick it up, or if he wants to go work with the 
> submitter, who may not have a clear understanding what the delay was 
> about or how the development process works, then he'd first need to 
> figure out what the status was, right?
>
> This sort of "facilitating" role seems like a useful thing to do
to
> allow contributions which would otherwise flounder to make it above the 
> bar into acceptance.
A facilitator should first ask the submitter what they think is going
on, and try and offer them guidance, which could be fairly generic e.g.
remind them that the need to resubmit if a patch is dropped, or it could
be more specific, i.e. reminding them of the need to respond to
particular queries made by the maintainers last time.

Yes, this means they would have needed to have read the background and
make an attempt to understand the current status first. They may not be
able to figure it out (or might get it wrong) but at least the obvious
cases can then be covered.

Facilitating can surely be useful and appreciated. But if "facilitate"
just means pinging the maintainers without having spoken to the
submitter, read up on the background and followed up on outstanding
queries then it is just making more work for maintainers for no gain and
isn't really facilitating anything.

Remember, Submitters (and facilitators) scale while maintainers do not.

Ian.



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

>>> On 10.05.13 at 12:09, George Dunlap
<george.dunlap@eu.citrix.com> wrote:
> But if he were going to pick it up, or if he wants to go work with the 
> submitter, who may not have a clear understanding what the delay was 
> about or how the development process works, then he''d first need
to
> figure out what the status was, right?
But that''s what is available in the list archives - in the case at
hand,
Pasi pointed at patches that had trees of subsequent communication
attached to them, and in all cases the leaves where unanswered
questions. So figuring out the status was quite strait forward here.

I admit that this may not always be as simple (or otherwise I likely
wouldn''t even have bothered looking at the old conversation again).

Bottom line is that I think we should be allowed to expect homework
to be done before pinging maintainers.

Jan

On Fri, May 10, 2013 at 11:33:09AM +0100, Ian Campbell
wrote:
> > >> I mostly wanted to clarify the situation with these patches,
> > > Instead of adding more work to our already overburdened
maintainers
> > > please start in these cases by working with the submitter.
> > >
> > > It is the submitters responsibility to respond to queries and to
keep
> > > pushing the patch as necessary. If the submitter isn''t
engaged any more
> > > then you need to either pick it up yourself or find someone else
who is
> > > willing to carry the patch forward, there''s no point
asking the
> > > maintainers to clarify the situation if no one is around who is
going to
> > > act on their responses.
> > 
> > But if he were going to pick it up, or if he wants to go work with the
> > submitter, who may not have a clear understanding what the delay was 
> > about or how the development process works, then he''d first
need to
> > figure out what the status was, right?
> >
> > This sort of "facilitating" role seems like a useful thing
to do to
> > allow contributions which would otherwise flounder to make it above
the
> > bar into acceptance.
> 
> A facilitator should first ask the submitter what they think is going
> on, and try and offer them guidance, which could be fairly generic e.g.
> remind them that the need to resubmit if a patch is dropped, or it could
> be more specific, i.e. reminding them of the need to respond to
> particular queries made by the maintainers last time.
> 
Yep, I should have talked to G.R. first. Sorry about that.
> Yes, this means they would have needed to have read the background and
> make an attempt to understand the current status first. They may not be
> able to figure it out (or might get it wrong) but at least the obvious
> cases can then be covered.
> 
I did read all the emails in various threads related to these patches,
and I tried understanding where we are.

It was a bit confusing because for example one of the patches got applied,
then reverted, re-submitted after that, and I didn''t 100% understand
from
the comments if there are still unresolved issues or not. 

Now it''s clear that not all of the issues are sorted out yet.

(Btw right now we''re discussing in a thread that doesn''t have
the latest version
of the patches..)
> Facilitating can surely be useful and appreciated. But if
"facilitate"
> just means pinging the maintainers without having spoken to the
> submitter, read up on the background and followed up on outstanding
> queries then it is just making more work for maintainers for no gain and
> isn''t really facilitating anything.
> 
> Remember, Submitters (and facilitators) scale while maintainers do not.
> 
I do understand that. Thanks for your patience :)

-- Pasi