* Re: virtio scsi host draft specification, v3
From: Hai Dong,Li @ 2011-07-04 13:38 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Christoph Hellwig, Stefan Hajnoczi, kvm, Michael S. Tsirkin,
qemu-devel, Linux Kernel Mailing List, Linux Virtualization
In-Reply-To: <4E0D86E0.1020406@redhat.com>
On 07/01/2011 09:14 AM, Hannes Reinecke wrote:
>> Actually, the kernel does _not_ do a LUN remapping.
>
> Not the kernel, the in-kernel target. The in-kernel target can and will
> map hardware LUNs (target_lun in drivers/target/*) to arbitrary LUNs
> (mapped_lun).
>
>>> Put in another way: the virtio-scsi device is itself a SCSI
>>> target,
>>
>> Argl. No way. The virtio-scsi device has to map to a single LUN.
>
> I think we are talking about different things. By "virtio-scsi device"
> I meant the "virtio-scsi HBA". When I referred to a LUN as seen by the
> guest, I was calling it a "virtual SCSI device". So yes, we were
> calling things with different names. Perhaps from now on
> we can call them virtio-scsi {initiator,target,LUN} and have no
> ambiguity? I'll also modify the spec in this sense.
>
>> The SCSI spec itself only deals with LUNs, so anything you'll read in
>> there obviously will only handle the interaction between the
>> initiator (read: host) and the LUN itself. However, the actual
>> command is send via an intermediat target, hence you'll always see
>> the reference to the ITL (initiator-target-lun) nexus.
>
> Yes, this I understand.
>
>> The SCSI spec details discovery of the individual LUNs presented by a
>> given target, it does _NOT_ detail the discovery of the targets
>> themselves. That is being delegated to the underlying transport
>
> And in fact I have this in virtio-scsi too, since virtio-scsi _is_ a
> transport:
Oh, here I catch up. I was wondering why there're ordering issues when
talking
about virtio-scsi, since in SAM3, the third and the last paragraph of
section
4.6.3 Request/Response ordering clearly describe it:
The manner in which ordering constraints are established is vendor
specific. An
implementation may delegate this responsibility to the application client
(e.g., the device driver). In-order delivery may be an intrinsic property of
the service delivery subsystem or a requirement established by the SCSI
transport protocol standard.
To simplify the description of behavior, the SCSI architecture model assumes
in-order delivery of requests or responses to be a property of the service
delivery subsystem. This assumption does not constitute a requirement. The
SCSI architecture model makes no assumption about and places no
requirement on
the ordering of requests or responses for different I_T nexuses.
So if I understand correctly, virtio-scsi looks like an SCSI tranport
protocol,
such as iSCSI, FCP and SRP which use tcp/ip, FC and Infiniband RDMA
respectively as the transfer media while virtio-scsi uses virtio, an
virtual IO
channel, as the transfer media?
>
> When VIRTIO_SCSI_EVT_RESET_REMOVED or VIRTIO_SCSI_EVT_RESET_RESCAN
> is sent for LUN 0, the driver should ask the initiator to rescan
> the target, in order to detect the case when an entire target has
> appeared or disappeared.
>
> [If the device fails] to report an event due to missing buffers,
> [...] the driver should poll the logical units for unit attention
> conditions, and/or do whatever form of bus scan is appropriate for
> the guest operating system.
>
>> In the case of NPIV it would make sense to map the virtual SCSI host
>> to the backend, so that all devices presented to the virtual SCSI
>> host will be presented to the backend, too. However, when doing so
>> these devices will normally be referenced by their original LUN, as
>> these will be presented to the guest via eg 'REPORT LUNS'.
>
> Right.
>
>> The above thread now tries to figure out if we should remap those LUN
>> numbers or just expose them as they are. If we decide on remapping,
>> we have to emulate _all_ commands referring explicitely to those LUN
>> numbers (persistent reservations, anyone?).
>
> But it seems to me that commands referring explicitly to LUN numbers
> most likely have to be reimplemented anyway for virtualization. I'm
> thinking exactly of persistent reservations. If two guests on the same
> host try a persistent reservation, they should conflict with each other.
> If reservation commands were just passed through, they would be seen
> as coming from the same initiator (the HBA driver or iSCSI initiator in
> the host OS).
>
> etc.
>
>> If we don't, we would expose some hardware detail to the guest, but
>> would save us _a lot_ of processing.
>
> But can we afford it? And would the architecture allow that at all?
>
> Paolo
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: virtio scsi host draft specification, v3
From: Stefan Hajnoczi @ 2011-07-04 14:22 UTC (permalink / raw)
To: Hai Dong,Li
Cc: Christoph Hellwig, Stefan Hajnoczi, kvm, Michael S. Tsirkin,
qemu-devel, Linux Kernel Mailing List, Paolo Bonzini,
Linux Virtualization
In-Reply-To: <4E11C263.3010003@linux.vnet.ibm.com>
On Mon, Jul 4, 2011 at 2:38 PM, Hai Dong,Li <haidongl@linux.vnet.ibm.com> wrote:
> So if I understand correctly, virtio-scsi looks like an SCSI tranport
> protocol,
> such as iSCSI, FCP and SRP which use tcp/ip, FC and Infiniband RDMA
> respectively as the transfer media while virtio-scsi uses virtio, an virtual
> IO
> channel, as the transfer media?
Correct.
Stefan
^ permalink raw reply
* Re: [PATCH] Modpost section mismatch fix
From: Raghavendra D Prabhu @ 2011-07-04 22:16 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel,
Konrad Rzeszutek Wilk
In-Reply-To: <1309769388.634.51.camel@zakaz.uk.xensource.com>
* On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote:
>On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
>> [Sorry if duplicate, one earlier was corrupt]
>> Hi,
>> I got section mismatches reported by modpost in latest build. It got
>> reported for xen_register_pirq and xen_unplug_emulated_devices
>> functions.
>
>
>> xen_register_pirq makes reference to
>> acpi_sci_override_gsi in init.data section; marking
>> xen_register_pirq with __init is not feasible since calls are made
>> to it from acpi_register_gsi in non-init contexts. So marking it
>> __refdata based on assumption that when acpi_sci_override_gsi is
>> referenced, it is in early stages where it is alive.
>
>I don't think this assumption holds, since xen_register_pirq can be
>called at any time and basically unconditionally references
>acpi_sci_override_gsi.
Yeah, that has been my guess as well, however I am not privy to the
inner workings of Xen, so was not sure.
>
>If we don't want to remove the __init from acpi_sci_override_gsi then
>perhaps xen_setup_acpi_sci needs to stash it somewhere?
>
>Or maybe xen_register_pirq could take an "int force_irq" which, if not
>-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
>(actually via xen_register_gsi so the param would need to be propagated
>there) would be the only actual user?
xen_register_gsi and hence, xen_register_pirq are called from
init (with xen_setup_acpi_sci) and non-init (with
acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
acpi_sci_override_gsi and is marked __init, the best way would be to
call xen_register_gsi and xen_register_pirq with a boolean argument like
sci_override to obviate the need to use acpi_sci_override_gsi in
register_pirq. I will send the patch with this change if it looks good.
>
>The xen_unplug_emulated_devices change looks correct to me since
>xen_unplug_emulated_devices is called from xen_arch_hvm_post_suspend.
>
>Ian.
>
>> --------------------------
>> Raghavendra Prabhu
>> GPG Id : 0xD72BE977
>> Fingerprint: B93F EBCB 8E05 7039 CD3C A4B8 A616 DCA1 D72B E977
>> www: wnohang.net
>> _______________________________________________
>> Virtualization mailing list
>> Virtualization@lists.linux-foundation.org
>> https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* [RFC] virtio: expose for non-virtualization users too
From: Ohad Ben-Cohen @ 2011-07-05 14:06 UTC (permalink / raw)
To: Rusty Russell, virtualization
Cc: Carsten Otte, linux-ia64, kvm, Michael S. Tsirkin,
Benjamin Herrenschmidt, linux-mips, Heiko Carstens, Grant Likely,
Paul Mackerras, H. Peter Anvin, linux-s390, linux-sh, x86,
Christian Borntraeger, Ingo Molnar, Avi Kivity, Russell King,
Fenghua Yu, Arnd Bergmann, Chris Metcalf, John Stultz, kvm-ppc,
Thomas Gleixner, linux-omap@
virtio has been so far used only in the context of virtualization,
and the virtio Kconfig was sourced directly by the relevant arch
Kconfigs when VIRTUALIZATION was selected.
Now that we start using virtio for inter-processor communications,
we need to source the virtio Kconfig outside of the virtualization
scope too.
Moreover, some architectures might use virtio for both virtualization
and inter-processor communications, so directly sourcing virtio
might yield unexpected results due to conflicting selections.
The simple solution offered by this patch is to always source virtio's
Kconfig in drivers/Kconfig, and remove it from the appropriate arch
Kconfigs. Additionally, a virtio menu entry has been added so virtio
drivers don't show up in the general drivers menu.
This way anyone can use virtio, though it's arguably less accessible
(and neat!) for virtualization users now.
Note: some architectures (mips and sh) seem to have a VIRTUALIZATION
menu merely for sourcing virtio's Kconfig, so that menu is removed too.
Signed-off-by: Ohad Ben-Cohen <ohad@wizery.com>
---
The motivation behind this patch is: https://lkml.org/lkml/2011/6/21/47
If the general approach is agreed upon, we can either merge the patch
independently, or add it to the AMP patch set.
arch/ia64/kvm/Kconfig | 1 -
arch/mips/Kconfig | 16 ----------------
arch/powerpc/kvm/Kconfig | 1 -
arch/s390/kvm/Kconfig | 1 -
arch/sh/Kconfig | 16 ----------------
arch/tile/kvm/Kconfig | 1 -
arch/x86/kvm/Kconfig | 1 -
drivers/Kconfig | 2 ++
drivers/virtio/Kconfig | 3 +++
9 files changed, 5 insertions(+), 37 deletions(-)
diff --git a/arch/ia64/kvm/Kconfig b/arch/ia64/kvm/Kconfig
index fa4d1e5..9806e55 100644
--- a/arch/ia64/kvm/Kconfig
+++ b/arch/ia64/kvm/Kconfig
@@ -49,6 +49,5 @@ config KVM_INTEL
extensions.
source drivers/vhost/Kconfig
-source drivers/virtio/Kconfig
endif # VIRTUALIZATION
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 653da62..a627a2c 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -2489,20 +2489,4 @@ source "security/Kconfig"
source "crypto/Kconfig"
-menuconfig VIRTUALIZATION
- bool "Virtualization"
- default n
- ---help---
- Say Y here to get to see options for using your Linux host to run other
- operating systems inside virtual machines (guests).
- This option alone does not add any kernel code.
-
- If you say N, all options in this submenu will be skipped and disabled.
-
-if VIRTUALIZATION
-
-source drivers/virtio/Kconfig
-
-endif # VIRTUALIZATION
-
source "lib/Kconfig"
diff --git a/arch/powerpc/kvm/Kconfig b/arch/powerpc/kvm/Kconfig
index b7baff7..105b691 100644
--- a/arch/powerpc/kvm/Kconfig
+++ b/arch/powerpc/kvm/Kconfig
@@ -99,6 +99,5 @@ config KVM_E500
If unsure, say N.
source drivers/vhost/Kconfig
-source drivers/virtio/Kconfig
endif # VIRTUALIZATION
diff --git a/arch/s390/kvm/Kconfig b/arch/s390/kvm/Kconfig
index f66a1bd..a216341 100644
--- a/arch/s390/kvm/Kconfig
+++ b/arch/s390/kvm/Kconfig
@@ -37,6 +37,5 @@ config KVM
# OK, it's a little counter-intuitive to do this, but it puts it neatly under
# the virtualization menu.
source drivers/vhost/Kconfig
-source drivers/virtio/Kconfig
endif # VIRTUALIZATION
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index bbdeb48..748ff19 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -897,20 +897,4 @@ source "security/Kconfig"
source "crypto/Kconfig"
-menuconfig VIRTUALIZATION
- bool "Virtualization"
- default n
- ---help---
- Say Y here to get to see options for using your Linux host to run other
- operating systems inside virtual machines (guests).
- This option alone does not add any kernel code.
-
- If you say N, all options in this submenu will be skipped and disabled.
-
-if VIRTUALIZATION
-
-source drivers/virtio/Kconfig
-
-endif # VIRTUALIZATION
-
source "lib/Kconfig"
diff --git a/arch/tile/kvm/Kconfig b/arch/tile/kvm/Kconfig
index b88f9c0..669fcdb 100644
--- a/arch/tile/kvm/Kconfig
+++ b/arch/tile/kvm/Kconfig
@@ -33,6 +33,5 @@ config KVM
If unsure, say N.
source drivers/vhost/Kconfig
-source drivers/virtio/Kconfig
endif # VIRTUALIZATION
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 50f6364..65cf823 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -76,6 +76,5 @@ config KVM_MMU_AUDIT
# the virtualization menu.
source drivers/vhost/Kconfig
source drivers/lguest/Kconfig
-source drivers/virtio/Kconfig
endif # VIRTUALIZATION
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 3bb154d..795218e 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -114,6 +114,8 @@ source "drivers/uio/Kconfig"
source "drivers/vlynq/Kconfig"
+source "drivers/virtio/Kconfig"
+
source "drivers/xen/Kconfig"
source "drivers/staging/Kconfig"
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
index 3dd6294..57e493b 100644
--- a/drivers/virtio/Kconfig
+++ b/drivers/virtio/Kconfig
@@ -7,6 +7,8 @@ config VIRTIO_RING
tristate
depends on VIRTIO
+menu "Virtio drivers"
+
config VIRTIO_PCI
tristate "PCI driver for virtio devices (EXPERIMENTAL)"
depends on PCI && EXPERIMENTAL
@@ -33,3 +35,4 @@ config VIRTIO_BALLOON
If unsure, say M.
+endmenu
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] Modpost section mismatch fix
From: Konrad Rzeszutek Wilk @ 2011-07-05 14:13 UTC (permalink / raw)
To: Raghavendra D Prabhu
Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel,
Ian Campbell
In-Reply-To: <20110704221646.GB4490@Xye>
On Tue, Jul 05, 2011 at 03:46:46AM +0530, Raghavendra D Prabhu wrote:
> * On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote:
> >On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
> >>[Sorry if duplicate, one earlier was corrupt]
>
> >>Hi,
> >> I got section mismatches reported by modpost in latest build. It got
> >> reported for xen_register_pirq and xen_unplug_emulated_devices
> >> functions.
> >
> >
> >> xen_register_pirq makes reference to
> >> acpi_sci_override_gsi in init.data section; marking
> >> xen_register_pirq with __init is not feasible since calls are made
> >> to it from acpi_register_gsi in non-init contexts. So marking it
> >> __refdata based on assumption that when acpi_sci_override_gsi is
> >> referenced, it is in early stages where it is alive.
> >
> >I don't think this assumption holds, since xen_register_pirq can be
> >called at any time and basically unconditionally references
> >acpi_sci_override_gsi.
>
> Yeah, that has been my guess as well, however I am not privy to the
> inner workings of Xen, so was not sure.
> >
> >If we don't want to remove the __init from acpi_sci_override_gsi then
> >perhaps xen_setup_acpi_sci needs to stash it somewhere?
> >
> >Or maybe xen_register_pirq could take an "int force_irq" which, if not
> >-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
> >(actually via xen_register_gsi so the param would need to be propagated
> >there) would be the only actual user?
>
> xen_register_gsi and hence, xen_register_pirq are called from
> init (with xen_setup_acpi_sci) and non-init (with
> acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
> acpi_sci_override_gsi and is marked __init, the best way would be to
> call xen_register_gsi and xen_register_pirq with a boolean argument like
> sci_override to obviate the need to use acpi_sci_override_gsi in
> register_pirq. I will send the patch with this change if it looks good.
Hold on, let me rebase #stable/pci.cleanups and see if the issue
here disappears.
^ permalink raw reply
* Re: [TOME] Re: [PATCH] Modpost section mismatch fix
From: Raghavendra D Prabhu @ 2011-07-05 14:27 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel,
Ian Campbell
In-Reply-To: <20110705141323.GG2972@dumpdata.com>
[-- Attachment #1.1: Type: text/plain, Size: 2260 bytes --]
* On Tue, Jul 05, 2011 at 10:13:23AM -0400, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
>On Tue, Jul 05, 2011 at 03:46:46AM +0530, Raghavendra D Prabhu wrote:
>> * On Mon, Jul 04, 2011 at 09:49:48AM +0100, Ian Campbell <ijc@hellion.org.uk> wrote:
>> >On Mon, 2011-07-04 at 04:55 +0530, Raghavendra D Prabhu wrote:
>> >>[Sorry if duplicate, one earlier was corrupt]
>> >>Hi,
>> >> I got section mismatches reported by modpost in latest build. It got
>> >> reported for xen_register_pirq and xen_unplug_emulated_devices
>> >> functions.
>> >> xen_register_pirq makes reference to
>> >> acpi_sci_override_gsi in init.data section; marking
>> >> xen_register_pirq with __init is not feasible since calls are made
>> >> to it from acpi_register_gsi in non-init contexts. So marking it
>> >> __refdata based on assumption that when acpi_sci_override_gsi is
>> >> referenced, it is in early stages where it is alive.
>> >I don't think this assumption holds, since xen_register_pirq can be
>> >called at any time and basically unconditionally references
>> >acpi_sci_override_gsi.
>> Yeah, that has been my guess as well, however I am not privy to the
>> inner workings of Xen, so was not sure.
>> >If we don't want to remove the __init from acpi_sci_override_gsi then
>> >perhaps xen_setup_acpi_sci needs to stash it somewhere?
>> >Or maybe xen_register_pirq could take an "int force_irq" which, if not
>> >-1, would force a particular IRQ. The callsite in xen_setup_acpi_sci
>> >(actually via xen_register_gsi so the param would need to be propagated
>> >there) would be the only actual user?
>> xen_register_gsi and hence, xen_register_pirq are called from
>> init (with xen_setup_acpi_sci) and non-init (with
>> acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
>> acpi_sci_override_gsi and is marked __init, the best way would be to
>> call xen_register_gsi and xen_register_pirq with a boolean argument like
>> sci_override to obviate the need to use acpi_sci_override_gsi in
>> register_pirq. I will send the patch with this change if it looks good.
>
>Hold on, let me rebase #stable/pci.cleanups and see if the issue
>here disappears.
Thanks, will wait until the rebase and test after that.
[-- Attachment #1.2: Type: application/pgp-signature, Size: 490 bytes --]
[-- Attachment #2: Type: text/plain, Size: 184 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [TOME] Re: [PATCH] Modpost section mismatch fix
From: Konrad Rzeszutek Wilk @ 2011-07-05 14:48 UTC (permalink / raw)
To: Raghavendra D Prabhu
Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel,
Ian Campbell
In-Reply-To: <20110705142731.GA4463@Xye>
> >>xen_register_gsi and hence, xen_register_pirq are called from
> >>init (with xen_setup_acpi_sci) and non-init (with
> >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
> >>acpi_sci_override_gsi and is marked __init, the best way would be to
> >>call xen_register_gsi and xen_register_pirq with a boolean argument like
> >>sci_override to obviate the need to use acpi_sci_override_gsi in
> >>register_pirq. I will send the patch with this change if it looks good.
> >
> >Hold on, let me rebase #stable/pci.cleanups and see if the issue
> >here disappears.
> Thanks, will wait until the rebase and test after that.
Hm, it actually looks like it wont do the trick. Why don't you send
a patch against 3.0-rc6 with the outlined mechanism mentioned above.
^ permalink raw reply
* Re: [PATCH 75/77] Staging: hv: mouse: Disable auto-loading of the mouse driver
From: Greg KH @ 2011-07-05 15:57 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1308255470-3826-75-git-send-email-kys@microsoft.com>
On Thu, Jun 16, 2011 at 01:17:48PM -0700, K. Y. Srinivasan wrote:
> Disable auto-loading of the mouse driver.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> ---
> drivers/staging/hv/hv_mouse.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
> index 1b30f26..ab5f1da 100644
> --- a/drivers/staging/hv/hv_mouse.c
> +++ b/drivers/staging/hv/hv_mouse.c
> @@ -950,7 +950,7 @@ static void __exit mousevsc_exit(void)
>
> MODULE_LICENSE("GPL");
> MODULE_VERSION(HV_DRV_VERSION);
> -MODULE_ALIAS("vmbus:hv_mouse");
> +/*MODULE_ALIAS("vmbus:hv_mouse");*/
Why?
{hint, I think I know why, but you need to tell us why in the changelog
comment}
Dropped.
^ permalink raw reply
* Re: [PATCH 07/77] Staging: hv: vmbus: Introduce a function to map the dev_type guid to a name
From: Greg KH @ 2011-07-05 16:01 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1308255470-3826-7-git-send-email-kys@microsoft.com>
On Thu, Jun 16, 2011 at 01:16:40PM -0700, K. Y. Srinivasan wrote:
> In preparation for introducing machinery to support autoloading vmbus
> drivers, introduce a function to map the dev_type guid to a human readable name.
"Human readable"?
<snip>
> +static const char *blk_dev_type = "hv_block";
> +static const char *net_dev_type = "hv_net";
> +static const char *scsi_dev_type = "hv_scsi";
> +static const char *mouse_dev_type = "hv_mouse";
> +static const char *util_dev_type = "hv_util";
It looks like these are the module names, which aren't the most "human
readable" around.
> +/*
> + * Map the dev_type guid to a human readable string for setting
> + * up module aliases. The indices used in this function are based on
> + * the table defined earlier - supported_device_classes[]
> + */
> +const char *hv_get_devtype_name(const struct hv_guid *type)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_NUM_DEVICE_CLASSES_SUPPORTED; i++) {
> + if (!memcmp(type, supported_device_classes[i].data,
> + sizeof(struct hv_guid))) {
> + switch (i) {
> + case 0:
> + return scsi_dev_type;
> + case 1:
> + return net_dev_type;
> + case 2:
> + return mouse_dev_type;
> + case 3:
> + return blk_dev_type;
> + }
> + }
> + }
> + /*
> + * Currently the util driver is used
> + * to handle all these devices.
> + */
> + return util_dev_type;
> +}
Ok, you create a function to return a string, but I'm really afraid of
what you are going to do in the next patch with this...
^ permalink raw reply
* Re: [PATCH 09/77] Staging: hv: blkvsc: Use the newly introduced vmbus ID in the blockvsc driver
From: Greg KH @ 2011-07-05 16:04 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1308255470-3826-9-git-send-email-kys@microsoft.com>
On Thu, Jun 16, 2011 at 01:16:42PM -0700, K. Y. Srinivasan wrote:
> Setup the newly introduced id_table for the blkvsc driver.:
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> ---
> drivers/staging/hv/blkvsc_drv.c | 5 +++++
> 1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> index 3612574..5842db8 100644
> --- a/drivers/staging/hv/blkvsc_drv.c
> +++ b/drivers/staging/hv/blkvsc_drv.c
> @@ -802,10 +802,15 @@ static void blkvsc_request(struct request_queue *queue)
> }
> }
>
> +static const struct hv_vmbus_device_id id_table[] = {
> + { "hv_block" },
> + { "" }
> +};
Ick. Yeah, I was afraid of this :(
You are trying to use a "human" name here, which you could just use the
macro for the module name.
But, we don't care about the module name, we just care about what GUID
this driver says it supports. So put that in the driver, then use the
module alias for that GUID and have modprobe figure out what module to
load based on when the GUID is seen by the vmbus and exposed to
userspace through the hotplug event.
Think of the GUID as a PCI vendor/device/class signature, that's all it
is. None of this wierd "names" that you have to keep track of in the
core, and in the driver, and hope to keep them all in sync all the time.
So no, this isn't the way to do this sorry, please redo it like
PCI/USB/the entire rest of the kernel does it.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 15/77] Staging: hv: blkvsc: Add the appropriate MODULE_ALIAS() line
From: Greg KH @ 2011-07-05 16:06 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Hank Janssen
In-Reply-To: <1308255470-3826-15-git-send-email-kys@microsoft.com>
On Thu, Jun 16, 2011 at 01:16:48PM -0700, K. Y. Srinivasan wrote:
> Add the appropriate MODULE_ALIAS() line to support auto-loading.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> ---
> drivers/staging/hv/blkvsc_drv.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> index 5842db8..9496abe 100644
> --- a/drivers/staging/hv/blkvsc_drv.c
> +++ b/drivers/staging/hv/blkvsc_drv.c
> @@ -1027,5 +1027,6 @@ static void __exit blkvsc_exit(void)
> MODULE_LICENSE("GPL");
> MODULE_VERSION(HV_DRV_VERSION);
> MODULE_DESCRIPTION("Microsoft Hyper-V virtual block driver");
> +MODULE_ALIAS("vmbus:hv_block");
No, these should be automagically generated with the MODULE_DEVICE_ID()
macro that you use in the module with the GUID there, instead of this.
Don't look at platform drivers as any semblance of how you should ever
do anything (I'm guessing you just copied their way of doing things
here). You have a real bus here (or at least a half-way version of a
bus) that can autodetect devices and tell userspace that you now have
them. Use that, don't try to model the code after the old, broken, and
hopefully-someday-in-my-dreams-will-go-away platform code.
greg k-h
^ permalink raw reply
* Re: [PATCH 5/5] staging: hv: fix a kernel warning in netvsc_linkstatus_callback()
From: Greg KH @ 2011-07-05 16:10 UTC (permalink / raw)
To: Haiyang Zhang
Cc: hjanssen, kys, v-abkane, gregkh, linux-kernel, devel,
virtualization, stable
In-Reply-To: <1308322688-9796-5-git-send-email-haiyangz@microsoft.com>
On Fri, Jun 17, 2011 at 07:58:08AM -0700, Haiyang Zhang wrote:
> netif_notify_peers() is not allowed in netvsc_linkstatus_callback() on some
> distros,
"distros"? Huh? You mean kernel versions right?
And this doesn't matter for the current kernel, right? If not, then it
doesn't need to be applied. If so, then it should. Which is it?
> because netvsc_linkstatus_callback() is within IRQ context. So we
> move the first call to netif_notify_peers() into queued work as well, but with
> zero delay.
What happens if you don't have this patch applied? What breaks? Is it
just a warning? Something else?
Please resend this with more information.
greg k-h
^ permalink raw reply
* RE: [PATCH 75/77] Staging: hv: mouse: Disable auto-loading of the mouse driver
From: KY Srinivasan @ 2011-07-05 16:10 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <20110705155745.GA31998@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, July 05, 2011 11:58 AM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 75/77] Staging: hv: mouse: Disable auto-loading of the
> mouse driver
>
> On Thu, Jun 16, 2011 at 01:17:48PM -0700, K. Y. Srinivasan wrote:
> > Disable auto-loading of the mouse driver.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > ---
> > drivers/staging/hv/hv_mouse.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
> > index 1b30f26..ab5f1da 100644
> > --- a/drivers/staging/hv/hv_mouse.c
> > +++ b/drivers/staging/hv/hv_mouse.c
> > @@ -950,7 +950,7 @@ static void __exit mousevsc_exit(void)
> >
> > MODULE_LICENSE("GPL");
> > MODULE_VERSION(HV_DRV_VERSION);
> > -MODULE_ALIAS("vmbus:hv_mouse");
> > +/*MODULE_ALIAS("vmbus:hv_mouse");*/
>
> Why?
>
> {hint, I think I know why, but you need to tell us why in the changelog
> comment}
>
> Dropped.
I will re-submit this with appropriate comments. Have you also dropped
all remaining patches submitted in this patch-set as well as the patch-set
that I had submitted subsequently.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH 1/5] staging: hv: fix some white spaces in netvsc driver
From: Greg KH @ 2011-07-05 16:11 UTC (permalink / raw)
To: Haiyang Zhang
Cc: hjanssen, kys, v-abkane, gregkh, linux-kernel, devel,
virtualization
In-Reply-To: <1308322688-9796-1-git-send-email-haiyangz@microsoft.com>
On Fri, Jun 17, 2011 at 07:58:04AM -0700, Haiyang Zhang wrote:
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> ---
> drivers/staging/hv/netvsc.c | 8 ++++----
> drivers/staging/hv/netvsc_drv.c | 2 +-
> 2 files changed, 5 insertions(+), 5 deletions(-)
This is the only patch in the series that would apply (well, I didn't
try the 5/5 patch for the reasons stated there.)
Please redo these against the latest staging-next branch if you want
them applied.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 34/40] Staging: hv: storvsc: Add the contents of hyperv_storage.h to storvsc_drv.c
From: Greg KH @ 2011-07-05 16:12 UTC (permalink / raw)
To: KY Srinivasan
Cc: Christoph Hellwig, gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Abhishek Kane (Mindtree Consulting PVT LTD),
Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480816C44C@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Fri, Jul 01, 2011 at 12:48:44PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Christoph Hellwig [mailto:hch@infradead.org]
> > Sent: Friday, July 01, 2011 4:16 AM
> > To: KY Srinivasan
> > Cc: Christoph Hellwig; gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> > Abhishek Kane (Mindtree Consulting PVT LTD); Hank Janssen
> > Subject: Re: [PATCH 34/40] Staging: hv: storvsc: Add the contents of
> > hyperv_storage.h to storvsc_drv.c
> >
> > On Thu, Jun 30, 2011 at 08:13:51PM +0000, KY Srinivasan wrote:
> > > > > Add the contents of hyperv_storage.h to storvsc_drv.c and cleanup
> > > > storvsc_drv.c.n
> > > >
> > > > I'd at least leave the first half of the header that defines the
> > > > protocol around.
> > >
> > > I only got rid of the block comment at the start of hyperv_storage.h
> > > and consolidated the include files. Nothing of substance was deleted.
> >
> > I meant to say keeping a separate header file for the protocol defintion
> > might be a good idea. That way it is kept separate from the
> > implementation details
> >
> Ok; I get it. Although, a while ago as I was cleaning up the header files
> In preparation to exiting staging, Greg had indicated that each driver
> should, if possible be a single file. I merged the relevant files to comply
> with that directive.
Well, do what makes sense.
^ permalink raw reply
* Re: [PATCH 01/40] Staging: hv: storvsc: Do not aquire an unnecessary reference on stor_device
From: Greg KH @ 2011-07-05 16:14 UTC (permalink / raw)
To: K. Y. Srinivasan
Cc: gregkh, linux-kernel, devel, virtualization, Haiyang Zhang,
Abhishek Kane
In-Reply-To: <1309358377-8537-1-git-send-email-kys@microsoft.com>
On Wed, Jun 29, 2011 at 07:38:58AM -0700, K. Y. Srinivasan wrote:
> On entry into storvsc_on_io_completion() we have already acquired a reference
> on the stor_device; there is no need to acquire an additional reference here.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
This series doesn't apply anymore, care to redo it?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 75/77] Staging: hv: mouse: Disable auto-loading of the mouse driver
From: Greg KH @ 2011-07-05 16:18 UTC (permalink / raw)
To: KY Srinivasan
Cc: Greg KH, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480816CCA8@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Tue, Jul 05, 2011 at 04:10:46PM +0000, KY Srinivasan wrote:
>
>
> > -----Original Message-----
> > From: Greg KH [mailto:greg@kroah.com]
> > Sent: Tuesday, July 05, 2011 11:58 AM
> > To: KY Srinivasan
> > Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> > devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> > Janssen
> > Subject: Re: [PATCH 75/77] Staging: hv: mouse: Disable auto-loading of the
> > mouse driver
> >
> > On Thu, Jun 16, 2011 at 01:17:48PM -0700, K. Y. Srinivasan wrote:
> > > Disable auto-loading of the mouse driver.
> > >
> > > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > > ---
> > > drivers/staging/hv/hv_mouse.c | 2 +-
> > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/drivers/staging/hv/hv_mouse.c b/drivers/staging/hv/hv_mouse.c
> > > index 1b30f26..ab5f1da 100644
> > > --- a/drivers/staging/hv/hv_mouse.c
> > > +++ b/drivers/staging/hv/hv_mouse.c
> > > @@ -950,7 +950,7 @@ static void __exit mousevsc_exit(void)
> > >
> > > MODULE_LICENSE("GPL");
> > > MODULE_VERSION(HV_DRV_VERSION);
> > > -MODULE_ALIAS("vmbus:hv_mouse");
> > > +/*MODULE_ALIAS("vmbus:hv_mouse");*/
> >
> > Why?
> >
> > {hint, I think I know why, but you need to tell us why in the changelog
> > comment}
> >
> > Dropped.
>
> I will re-submit this with appropriate comments. Have you also dropped
> all remaining patches submitted in this patch-set as well as the patch-set
> that I had submitted subsequently.
I have now applied all of the drivers/staging/hv/ patches in my
"to-apply" queue that I am going to apply, so feel free to rebase now
and redo and resend.
greg k-h
^ permalink raw reply
* RE: [PATCH 01/40] Staging: hv: storvsc: Do not aquire an unnecessary reference on stor_device
From: KY Srinivasan @ 2011-07-05 16:43 UTC (permalink / raw)
To: Greg KH
Cc: Abhishek Kane (Mindtree Consulting PVT LTD), Haiyang Zhang,
gregkh@suse.de, linux-kernel@vger.kernel.org,
virtualization@lists.osdl.org, devel@linuxdriverproject.org
In-Reply-To: <20110705161408.GA25532@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, July 05, 2011 12:14 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang;
> Abhishek Kane (Mindtree Consulting PVT LTD)
> Subject: Re: [PATCH 01/40] Staging: hv: storvsc: Do not aquire an unnecessary
> reference on stor_device
>
> On Wed, Jun 29, 2011 at 07:38:58AM -0700, K. Y. Srinivasan wrote:
> > On entry into storvsc_on_io_completion() we have already acquired a
> reference
> > on the stor_device; there is no need to acquire an additional reference here.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Abhishek Kane <v-abkane@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
>
> This series doesn't apply anymore, care to redo it?
Will do.
K. Y
^ permalink raw reply
* RE: [PATCH 5/5] staging: hv: fix a kernel warning in netvsc_linkstatus_callback()
From: Haiyang Zhang @ 2011-07-05 18:45 UTC (permalink / raw)
To: Greg KH
Cc: Hank Janssen, KY Srinivasan,
Abhishek Kane (Mindtree Consulting PVT LTD), gregkh@suse.de,
linux-kernel@vger.kernel.org, devel@linuxdriverproject.org,
virtualization@lists.osdl.org, stable
In-Reply-To: <20110705161007.GA24332@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, July 05, 2011 12:10 PM
> To: Haiyang Zhang
> Cc: Hank Janssen; KY Srinivasan; Abhishek Kane (Mindtree Consulting PVT
> LTD); gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; stable
> Subject: Re: [PATCH 5/5] staging: hv: fix a kernel warning in
> netvsc_linkstatus_callback()
>
> On Fri, Jun 17, 2011 at 07:58:08AM -0700, Haiyang Zhang wrote:
> > netif_notify_peers() is not allowed in netvsc_linkstatus_callback() on
> some
> > distros,
>
> "distros"? Huh? You mean kernel versions right?
>
> And this doesn't matter for the current kernel, right? If not, then it
> doesn't need to be applied. If so, then it should. Which is it?
It should be applied to both current kernel and stable kernels 2.6.32 and later.
> > because netvsc_linkstatus_callback() is within IRQ context. So we
> > move the first call to netif_notify_peers() into queued work as well,
> but with
> > zero delay.
>
> What happens if you don't have this patch applied? What breaks? Is it
> just a warning? Something else?
Without the patch, it will have this warning:
[ 232.102605] WARNING: at kernel/mutex.c:198 __mutex_lock_slowpath+0x28d/0x2c0(
)
[ 232.102605] Hardware name: Virtual Machine
[ 232.272436] Modules linked in: hv_netvsc(C) hv_vmbus(C) lockd sunrpc nf_connt
rack_netbios_ns nf_conntrack_broadcast ipt_REJECT nf_conntrack_ipv4 nf_defrag_ip
v4 xt_state nf_conntrack iptable_filter ip_tables ip6t_REJECT xt_tcpudp ip6table
_filter ip6_tables x_tables ipv6 loop dm_mirror dm_multipath scsi_dh video therm
al sbs sbshc pci_slot hed fan container battery acpi_memhotplug ac power_supply
parport nvram sg sr_mod cdrom evdev processor button serio_raw floppy tpm_tis tp
m tpm_bios tulip i2c_piix4 i2c_core dm_region_hash dm_log dm_mod ide_gd_mod piix
pata_acpi ide_pci_generic ide_core ata_piix ata_generic libata sd_mod scsi_mod
ext3 jbd mbcache [last unloaded: microcode]
[ 232.935109] Pid: 0, comm: swapper Tainted: G C 3.0.0-rc2-stable-0019
6-g06e8684 #4
[ 232.939905] Call Trace:
[ 232.942238] [<c13008fd>] ? __mutex_lock_slowpath+0x28d/0x2c0
[ 232.974990] [<c1041a9c>] warn_slowpath_common+0x7c/0xa0
[ 232.977701] [<c13008fd>] ? __mutex_lock_slowpath+0x28d/0x2c0
[ 232.980431] [<c1041add>] warn_slowpath_null+0x1d/0x20
[ 232.983403] [<c13008fd>] __mutex_lock_slowpath+0x28d/0x2c0
[ 232.995597] [<c130093b>] mutex_lock+0xb/0x20
[ 233.020191] [<c127347d>] rtnl_lock+0xd/0x10
[ 233.023275] [<c127cf7b>] netif_notify_peers+0xb/0x20
[ 233.026424] [<e0e581d2>] netvsc_linkstatus_callback+0x142/0x230 [hv_netvsc]
[ 233.051372] [<e0e5c0df>] rndis_filter_receive+0x83f/0x8e0 [hv_netvsc]
[ 233.062010] [<e0e3c82e>] ? hv_ringbuffer_read+0x15e/0x200 [hv_vmbus]
[ 233.065632] [<e0e5a936>] netvsc_channel_cb+0x8e6/0xb10 [hv_netvsc]
[ 233.069444] [<e0e38743>] vmbus_onchannel_event+0x33/0x80 [hv_vmbus]
[ 233.092295] [<e0e37bb8>] vmbus_on_event+0xf8/0x180 [hv_vmbus]
[ 233.104314] [<c104850f>] tasklet_action+0x4f/0x100
[ 233.106669] [<c1047876>] __do_softirq+0x86/0x1e0
[ 233.109376] [<c10477f0>] ? local_bh_enable_ip+0xa0/0xa0
[ 233.112031] <IRQ> [<c1047b6c>] ? irq_exit+0x8c/0xa0
[ 233.133300] [<c10040a5>] ? do_IRQ+0x45/0xb0
[ 233.136307] [<c1047b4e>] ? irq_exit+0x6e/0xa0
[ 233.138901] [<c101d149>] ? smp_apic_timer_interrupt+0x59/0x90
[ 233.142796] [<c1302db0>] ? common_interrupt+0x30/0x38
[ 233.146401] [<c1024be5>] ? native_safe_halt+0x5/0x10
[ 233.149765] [<c1009b00>] ? default_idle+0x70/0x130
[ 233.154054] [<c1001dda>] ? cpu_idle+0x8a/0xd0
[ 233.157170] [<c12eb4dd>] ? rest_init+0x7d/0x80
[ 233.168628] [<c14239e5>] ? start_kernel+0x315/0x3d0
[ 233.171518] [<c1423270>] ? parse_early_options+0x30/0x30
[ 233.180398] [<c1423078>] ? i386_start_kernel+0x78/0xc0
[ 233.183514] ---[ end trace 3a5b5bc7486ad570 ]---
> Please resend this with more information.
Will do (after Ky's patch series are resubmitted and applied).
Thanks,
- Haiyang
^ permalink raw reply
* Re: [TOME] Re: [PATCH] Modpost section mismatch fix
From: Konrad Rzeszutek Wilk @ 2011-07-05 21:32 UTC (permalink / raw)
To: Raghavendra D Prabhu
Cc: xen-devel, jeremy.fitzhardinge, virtualization, linux-kernel,
Ian Campbell
In-Reply-To: <20110705144846.GA13548@dumpdata.com>
On Tue, Jul 05, 2011 at 10:48:46AM -0400, Konrad Rzeszutek Wilk wrote:
> > >>xen_register_gsi and hence, xen_register_pirq are called from
> > >>init (with xen_setup_acpi_sci) and non-init (with
> > >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
> > >>acpi_sci_override_gsi and is marked __init, the best way would be to
> > >>call xen_register_gsi and xen_register_pirq with a boolean argument like
> > >>sci_override to obviate the need to use acpi_sci_override_gsi in
> > >>register_pirq. I will send the patch with this change if it looks good.
> > >
> > >Hold on, let me rebase #stable/pci.cleanups and see if the issue
> > >here disappears.
> > Thanks, will wait until the rebase and test after that.
>
> Hm, it actually looks like it wont do the trick. Why don't you send
> a patch against 3.0-rc6 with the outlined mechanism mentioned above.
Or this patch (against 3.0-rc6) might do the trick:
diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
index fe00830..f567965 100644
--- a/arch/x86/pci/xen.c
+++ b/arch/x86/pci/xen.c
@@ -327,13 +327,12 @@ int __init pci_xen_hvm_init(void)
}
#ifdef CONFIG_XEN_DOM0
-static int xen_register_pirq(u32 gsi, int triggering)
+static int xen_register_pirq(u32 gsi, int gsi_override, int triggering)
{
int rc, pirq, irq = -1;
struct physdev_map_pirq map_irq;
int shareable = 0;
char *name;
- bool gsi_override = false;
if (!xen_pv_domain())
return -1;
@@ -345,31 +344,12 @@ static int xen_register_pirq(u32 gsi, int triggering)
shareable = 1;
name = "ioapic-level";
}
-
pirq = xen_allocate_pirq_gsi(gsi);
if (pirq < 0)
goto out;
- /* Before we bind the GSI to a Linux IRQ, check whether
- * we need to override it with bus_irq (IRQ) value. Usually for
- * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
- * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
- * but there are oddballs where the IRQ != GSI:
- * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
- * which ends up being: gsi_to_irq[9] == 20
- * (which is what acpi_gsi_to_irq ends up calling when starting the
- * the ACPI interpreter and keels over since IRQ 9 has not been
- * setup as we had setup IRQ 20 for it).
- */
- if (gsi == acpi_sci_override_gsi) {
- /* Check whether the GSI != IRQ */
- acpi_gsi_to_irq(gsi, &irq);
- if (irq != gsi)
- /* Bugger, we MUST have that IRQ. */
- gsi_override = true;
- }
- if (gsi_override)
- irq = xen_bind_pirq_gsi_to_irq(irq, pirq, shareable, name);
+ if (gsi_override >= 0)
+ irq = xen_bind_pirq_gsi_to_irq(gsi_override, pirq, shareable, name);
else
irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name);
if (irq < 0)
@@ -392,7 +372,7 @@ out:
return irq;
}
-static int xen_register_gsi(u32 gsi, int triggering, int polarity)
+static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity)
{
int rc, irq;
struct physdev_setup_gsi setup_gsi;
@@ -403,7 +383,7 @@ static int xen_register_gsi(u32 gsi, int triggering, int polarity)
printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n",
gsi, triggering, polarity);
- irq = xen_register_pirq(gsi, triggering);
+ irq = xen_register_pirq(gsi, gsi_override, triggering);
setup_gsi.gsi = gsi;
setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1);
@@ -425,6 +405,8 @@ static __init void xen_setup_acpi_sci(void)
int rc;
int trigger, polarity;
int gsi = acpi_sci_override_gsi;
+ int irq = -1;
+ int gsi_override = -1;
if (!gsi)
return;
@@ -441,7 +423,25 @@ static __init void xen_setup_acpi_sci(void)
printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d "
"polarity=%d\n", gsi, trigger, polarity);
- gsi = xen_register_gsi(gsi, trigger, polarity);
+ /* Before we bind the GSI to a Linux IRQ, check whether
+ * we need to override it with bus_irq (IRQ) value. Usually for
+ * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
+ * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
+ * but there are oddballs where the IRQ != GSI:
+ * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
+ * which ends up being: gsi_to_irq[9] == 20
+ * (which is what acpi_gsi_to_irq ends up calling when starting the
+ * the ACPI interpreter and keels over since IRQ 9 has not been
+ * setup as we had setup IRQ 20 for it).
+ */
+ /* Check whether the GSI != IRQ */
+ if (acpi_gsi_to_irq(gsi, &irq) == 0) {
+ if (irq >= 0 && irq != gsi)
+ /* Bugger, we MUST have that IRQ. */
+ gsi_override = irq;
+ }
+
+ gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity);
printk(KERN_INFO "xen: acpi sci %d\n", gsi);
return;
@@ -450,7 +450,7 @@ static __init void xen_setup_acpi_sci(void)
static int acpi_register_gsi_xen(struct device *dev, u32 gsi,
int trigger, int polarity)
{
- return xen_register_gsi(gsi, trigger, polarity);
+ return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity);
}
static int __init pci_xen_initial_domain(void)
@@ -489,7 +489,7 @@ void __init xen_setup_pirqs(void)
if (acpi_get_override_irq(irq, &trigger, &polarity) == -1)
continue;
- xen_register_pirq(irq,
+ xen_register_pirq(irq, -1 /* no GSI override */,
trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE);
}
}
^ permalink raw reply related
* Re: [RFC] virtio: expose for non-virtualization users too
From: H. Peter Anvin @ 2011-07-05 22:35 UTC (permalink / raw)
To: Ohad Ben-Cohen
Cc: Carsten Otte, linux-ia64, kvm, Michael S. Tsirkin,
Benjamin Herrenschmidt, linux-mips, Heiko Carstens,
virtualization, Grant Likely, Paul Mackerras, linux-s390,
linux-sh, x86, Christian Borntraeger, Ingo Molnar, Avi Kivity,
Russell King, Fenghua Yu, Arnd Bergmann, Chris Metcalf,
John Stultz, kvm-ppc, Thomas Gleixner
In-Reply-To: <1309874774-31689-1-git-send-email-ohad@wizery.com>
On 07/05/2011 07:06 AM, Ohad Ben-Cohen wrote:
> virtio has been so far used only in the context of virtualization,
> and the virtio Kconfig was sourced directly by the relevant arch
> Kconfigs when VIRTUALIZATION was selected.
>
> Now that we start using virtio for inter-processor communications,
> we need to source the virtio Kconfig outside of the virtualization
> scope too.
>
Seems reasonable to me.
-hpa
^ permalink raw reply
* RE: [PATCH 15/77] Staging: hv: blkvsc: Add the appropriate MODULE_ALIAS() line
From: KY Srinivasan @ 2011-07-06 0:40 UTC (permalink / raw)
To: Greg KH
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <20110705160621.GC7214@kroah.com>
> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com]
> Sent: Tuesday, July 05, 2011 12:06 PM
> To: KY Srinivasan
> Cc: gregkh@suse.de; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; virtualization@lists.osdl.org; Haiyang Zhang; Hank
> Janssen
> Subject: Re: [PATCH 15/77] Staging: hv: blkvsc: Add the appropriate
> MODULE_ALIAS() line
>
> On Thu, Jun 16, 2011 at 01:16:48PM -0700, K. Y. Srinivasan wrote:
> > Add the appropriate MODULE_ALIAS() line to support auto-loading.
> >
> > Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> > Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
> > Signed-off-by: Hank Janssen <hjanssen@microsoft.com>
> > ---
> > drivers/staging/hv/blkvsc_drv.c | 1 +
> > 1 files changed, 1 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> > index 5842db8..9496abe 100644
> > --- a/drivers/staging/hv/blkvsc_drv.c
> > +++ b/drivers/staging/hv/blkvsc_drv.c
> > @@ -1027,5 +1027,6 @@ static void __exit blkvsc_exit(void)
> > MODULE_LICENSE("GPL");
> > MODULE_VERSION(HV_DRV_VERSION);
> > MODULE_DESCRIPTION("Microsoft Hyper-V virtual block driver");
> > +MODULE_ALIAS("vmbus:hv_block");
>
> No, these should be automagically generated with the MODULE_DEVICE_ID()
> macro that you use in the module with the GUID there, instead of this.
I think you mean MODULE_DEVICE_TABLE()? I actually went down that path first
adding code to file2alias.c for parsing the vmbus ID table. Given that this approach
would make it impossible to support auto-loading of these drivers
on many of the released kernels, I chose to go with the MODULE_ALIAS() macro
that did not need any changes outside our drivers. In both methods, the formatting
of the name is bus specific since I would be writing the code to parse the table in
file2alias.c.
Granted, I have been quite unimaginative in my alias names, but I thought they were
reasonably descriptive. If at all possible, for the reasons listed above, I would prefer to use
the MODULE_ALIAS() macro (I could embed all or part of the guid in the alias). Let me know.
Regards,
K. Y
^ permalink raw reply
* Re: [PATCH 15/77] Staging: hv: blkvsc: Add the appropriate MODULE_ALIAS() line
From: Greg KH @ 2011-07-06 3:42 UTC (permalink / raw)
To: KY Srinivasan
Cc: gregkh@suse.de, linux-kernel@vger.kernel.org,
devel@linuxdriverproject.org, virtualization@lists.osdl.org,
Haiyang Zhang, Hank Janssen
In-Reply-To: <6E21E5352C11B742B20C142EB499E0480816D27E@TK5EX14MBXC126.redmond.corp.microsoft.com>
On Wed, Jul 06, 2011 at 12:40:42AM +0000, KY Srinivasan wrote:
> > > diff --git a/drivers/staging/hv/blkvsc_drv.c b/drivers/staging/hv/blkvsc_drv.c
> > > index 5842db8..9496abe 100644
> > > --- a/drivers/staging/hv/blkvsc_drv.c
> > > +++ b/drivers/staging/hv/blkvsc_drv.c
> > > @@ -1027,5 +1027,6 @@ static void __exit blkvsc_exit(void)
> > > MODULE_LICENSE("GPL");
> > > MODULE_VERSION(HV_DRV_VERSION);
> > > MODULE_DESCRIPTION("Microsoft Hyper-V virtual block driver");
> > > +MODULE_ALIAS("vmbus:hv_block");
> >
> > No, these should be automagically generated with the MODULE_DEVICE_ID()
> > macro that you use in the module with the GUID there, instead of this.
>
> I think you mean MODULE_DEVICE_TABLE()?
Yes, sorry for the typo.
> I actually went down that path first
> adding code to file2alias.c for parsing the vmbus ID table. Given that this approach
> would make it impossible to support auto-loading of these drivers
> on many of the released kernels,
Wait, what? What is a "released kernel"? We are working on the
in-kernel patch, we don't care about older distros/releases for this
work at all. Also, it doesn't make sense at all, why would the change I
asked for make any difference on older distros/kernels?
> I chose to go with the MODULE_ALIAS() macro that did not need any
> changes outside our drivers. In both methods, the formatting of the
> name is bus specific since I would be writing the code to parse the
> table in file2alias.c.
Yes, that is what is needed to be done.
> Granted, I have been quite unimaginative in my alias names, but I
> thought they were reasonably descriptive. If at all possible, for the
> reasons listed above, I would prefer to use the MODULE_ALIAS() macro
> (I could embed all or part of the guid in the alias). Let me know.
Please do the correct thing and use MODULE_DEVICE_TABLE().
thanks,
greg k-h
^ permalink raw reply
* Re: [RFC] virtio: expose for non-virtualization users too
From: Rusty Russell @ 2011-07-06 5:25 UTC (permalink / raw)
To: H. Peter Anvin, Ohad Ben-Cohen; +Cc: linux-kernel, virtualization
In-Reply-To: <4E1391C2.2090904@zytor.com>
On Tue, 05 Jul 2011 15:35:46 -0700, "H. Peter Anvin" <hpa@zytor.com> wrote:
> On 07/05/2011 07:06 AM, Ohad Ben-Cohen wrote:
> > virtio has been so far used only in the context of virtualization,
> > and the virtio Kconfig was sourced directly by the relevant arch
> > Kconfigs when VIRTUALIZATION was selected.
> >
> > Now that we start using virtio for inter-processor communications,
> > we need to source the virtio Kconfig outside of the virtualization
> > scope too.
> >
>
> Seems reasonable to me.
Me too. When you're happy, please send.
Thanks,
Rusty.
^ permalink raw reply
* Re: [Xen-devel] Re: [TOME] Re: [PATCH] Modpost section mismatch fix
From: Ian Campbell @ 2011-07-06 8:30 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel@vger.kernel.org, Raghavendra D Prabhu,
xen-devel@lists.xensource.com, Jeremy Fitzhardinge,
virtualization@lists.linux-foundation.org
In-Reply-To: <20110705213243.GA3647@dumpdata.com>
On Tue, 2011-07-05 at 22:32 +0100, Konrad Rzeszutek Wilk wrote:
> On Tue, Jul 05, 2011 at 10:48:46AM -0400, Konrad Rzeszutek Wilk wrote:
> > > >>xen_register_gsi and hence, xen_register_pirq are called from
> > > >>init (with xen_setup_acpi_sci) and non-init (with
> > > >>acpi_register_gsi_xen); since xen_set_acpi_sci calls it with gsi ==
> > > >>acpi_sci_override_gsi and is marked __init, the best way would be to
> > > >>call xen_register_gsi and xen_register_pirq with a boolean argument like
> > > >>sci_override to obviate the need to use acpi_sci_override_gsi in
> > > >>register_pirq. I will send the patch with this change if it looks good.
> > > >
> > > >Hold on, let me rebase #stable/pci.cleanups and see if the issue
> > > >here disappears.
> > > Thanks, will wait until the rebase and test after that.
> >
> > Hm, it actually looks like it wont do the trick. Why don't you send
> > a patch against 3.0-rc6 with the outlined mechanism mentioned above.
>
> Or this patch (against 3.0-rc6) might do the trick:
Based on my limited understanding it looks like it would to me.
But is there some downside to always unconditionally calling
acpi_gsi_to_irq in xen_register_pirq? It seems like it returns the
expected mapping except where explicit overrides (such as this SCI
thing) exist?
Ian.
> diff --git a/arch/x86/pci/xen.c b/arch/x86/pci/xen.c
> index fe00830..f567965 100644
> --- a/arch/x86/pci/xen.c
> +++ b/arch/x86/pci/xen.c
> @@ -327,13 +327,12 @@ int __init pci_xen_hvm_init(void)
> }
>
> #ifdef CONFIG_XEN_DOM0
> -static int xen_register_pirq(u32 gsi, int triggering)
> +static int xen_register_pirq(u32 gsi, int gsi_override, int triggering)
> {
> int rc, pirq, irq = -1;
> struct physdev_map_pirq map_irq;
> int shareable = 0;
> char *name;
> - bool gsi_override = false;
>
> if (!xen_pv_domain())
> return -1;
> @@ -345,31 +344,12 @@ static int xen_register_pirq(u32 gsi, int triggering)
> shareable = 1;
> name = "ioapic-level";
> }
> -
> pirq = xen_allocate_pirq_gsi(gsi);
> if (pirq < 0)
> goto out;
>
> - /* Before we bind the GSI to a Linux IRQ, check whether
> - * we need to override it with bus_irq (IRQ) value. Usually for
> - * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
> - * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
> - * but there are oddballs where the IRQ != GSI:
> - * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
> - * which ends up being: gsi_to_irq[9] == 20
> - * (which is what acpi_gsi_to_irq ends up calling when starting the
> - * the ACPI interpreter and keels over since IRQ 9 has not been
> - * setup as we had setup IRQ 20 for it).
> - */
> - if (gsi == acpi_sci_override_gsi) {
> - /* Check whether the GSI != IRQ */
> - acpi_gsi_to_irq(gsi, &irq);
> - if (irq != gsi)
> - /* Bugger, we MUST have that IRQ. */
> - gsi_override = true;
> - }
> - if (gsi_override)
> - irq = xen_bind_pirq_gsi_to_irq(irq, pirq, shareable, name);
> + if (gsi_override >= 0)
> + irq = xen_bind_pirq_gsi_to_irq(gsi_override, pirq, shareable, name);
> else
> irq = xen_bind_pirq_gsi_to_irq(gsi, pirq, shareable, name);
> if (irq < 0)
> @@ -392,7 +372,7 @@ out:
> return irq;
> }
>
> -static int xen_register_gsi(u32 gsi, int triggering, int polarity)
> +static int xen_register_gsi(u32 gsi, int gsi_override, int triggering, int polarity)
> {
> int rc, irq;
> struct physdev_setup_gsi setup_gsi;
> @@ -403,7 +383,7 @@ static int xen_register_gsi(u32 gsi, int triggering, int polarity)
> printk(KERN_DEBUG "xen: registering gsi %u triggering %d polarity %d\n",
> gsi, triggering, polarity);
>
> - irq = xen_register_pirq(gsi, triggering);
> + irq = xen_register_pirq(gsi, gsi_override, triggering);
>
> setup_gsi.gsi = gsi;
> setup_gsi.triggering = (triggering == ACPI_EDGE_SENSITIVE ? 0 : 1);
> @@ -425,6 +405,8 @@ static __init void xen_setup_acpi_sci(void)
> int rc;
> int trigger, polarity;
> int gsi = acpi_sci_override_gsi;
> + int irq = -1;
> + int gsi_override = -1;
>
> if (!gsi)
> return;
> @@ -441,7 +423,25 @@ static __init void xen_setup_acpi_sci(void)
> printk(KERN_INFO "xen: sci override: global_irq=%d trigger=%d "
> "polarity=%d\n", gsi, trigger, polarity);
>
> - gsi = xen_register_gsi(gsi, trigger, polarity);
> + /* Before we bind the GSI to a Linux IRQ, check whether
> + * we need to override it with bus_irq (IRQ) value. Usually for
> + * IRQs below IRQ_LEGACY_IRQ this holds IRQ == GSI, as so:
> + * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
> + * but there are oddballs where the IRQ != GSI:
> + * ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 20 low level)
> + * which ends up being: gsi_to_irq[9] == 20
> + * (which is what acpi_gsi_to_irq ends up calling when starting the
> + * the ACPI interpreter and keels over since IRQ 9 has not been
> + * setup as we had setup IRQ 20 for it).
> + */
> + /* Check whether the GSI != IRQ */
> + if (acpi_gsi_to_irq(gsi, &irq) == 0) {
> + if (irq >= 0 && irq != gsi)
> + /* Bugger, we MUST have that IRQ. */
> + gsi_override = irq;
> + }
> +
> + gsi = xen_register_gsi(gsi, gsi_override, trigger, polarity);
> printk(KERN_INFO "xen: acpi sci %d\n", gsi);
>
> return;
> @@ -450,7 +450,7 @@ static __init void xen_setup_acpi_sci(void)
> static int acpi_register_gsi_xen(struct device *dev, u32 gsi,
> int trigger, int polarity)
> {
> - return xen_register_gsi(gsi, trigger, polarity);
> + return xen_register_gsi(gsi, -1 /* no GSI override */, trigger, polarity);
> }
>
> static int __init pci_xen_initial_domain(void)
> @@ -489,7 +489,7 @@ void __init xen_setup_pirqs(void)
> if (acpi_get_override_irq(irq, &trigger, &polarity) == -1)
> continue;
>
> - xen_register_pirq(irq,
> + xen_register_pirq(irq, -1 /* no GSI override */,
> trigger ? ACPI_LEVEL_SENSITIVE : ACPI_EDGE_SENSITIVE);
> }
> }
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
--
Ian Campbell
While having never invented a sin, I'm trying to perfect several.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox