work on your photos
We have got a team of professional to do image editing service. We have 8 image editors and on daily basis 300 images can be processed, Detail below: Photo cut out; Photo clipping path; Photo masking Photo shadow creation; Photo color correction Photo retouching; Beauty Model retouching on skin, face, body. Glamour retouching; Products retouching And other image editing We will provide you editing test on your photos. if you want to know more about us Please reply back. Thanks, Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] PCI: hv: use effective affinity mask
> -Original Message- > From: Dexuan Cui > Sent: Wednesday, November 1, 2017 1:31 PM > To: Bjorn Helgaas ; linux-...@vger.kernel.org; Jake > Oshins ; KY Srinivasan ; > Stephen Hemminger > Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; Haiyang > Zhang ; Jork Loeser > ; Chris Valean (Cloudbase Solutions SRL) chv...@microsoft.com>; Adrian Suhov (Cloudbase Solutions SRL) ads...@microsoft.com>; Simon Xiao ; 'Eyal > Mizrachi' ; Jack Morgenstein > ; Armen Guezalian ; Firas > Mahameed ; Tziporet Koren > ; Daniel Jurgens > Subject: [PATCH] PCI: hv: use effective affinity mask > > > The effective_affinity_mask is always set when an interrupt is assigned in > __assign_irq_vector() -> apic->cpu_mask_to_apicid(), e.g. for struct apic > apic_physflat: -> default_cpu_mask_to_apicid() -> > irq_data_update_effective_affinity(), but it looks d->common->affinity > remains all-1's before the user space or the kernel changes it later. > > In the early allocation/initialization phase of an irq, we should use the > effective_affinity_mask, otherwise Hyper-V may not deliver the interrupt to > the expected cpu. Without the patch, if we assign 7 Mellanox ConnectX-3 > VFs to a 32-vCPU VM, one of the VFs may fail to receive interrupts. > > Signed-off-by: Dexuan Cui > Cc: Jake Oshins > Cc: Jork Loeser > Cc: Stephen Hemminger > Cc: K. Y. Srinivasan > --- > > Please consider this for v4.14, if it's not too late. > > drivers/pci/host/pci-hyperv.c | 8 +--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 5ccb47d..8b5f66d 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -879,7 +879,7 @@ static void hv_irq_unmask(struct irq_data *data) > int cpu; > u64 res; > > - dest = irq_data_get_affinity_mask(data); > + dest = irq_data_get_effective_affinity_mask(data); > pdev = msi_desc_to_pci_dev(msi_desc); > pbus = pdev->bus; > hbus = container_of(pbus->sysdata, struct hv_pcibus_device, > sysdata); @@ -1042,6 +1042,7 @@ static void hv_compose_msi_msg(struct > irq_data *data, struct msi_msg *msg) > struct hv_pci_dev *hpdev; > struct pci_bus *pbus; > struct pci_dev *pdev; > + struct cpumask *dest; > struct compose_comp_ctxt comp; > struct tran_int_desc *int_desc; > struct { > @@ -1056,6 +1057,7 @@ static void hv_compose_msi_msg(struct irq_data > *data, struct msi_msg *msg) > int ret; > > pdev = msi_desc_to_pci_dev(irq_data_get_msi_desc(data)); > + dest = irq_data_get_effective_affinity_mask(data); > pbus = pdev->bus; > hbus = container_of(pbus->sysdata, struct hv_pcibus_device, > sysdata); > hpdev = get_pcichild_wslot(hbus, devfn_to_wslot(pdev->devfn)); > @@ -1081,14 +1083,14 @@ static void hv_compose_msi_msg(struct irq_data > *data, struct msi_msg *msg) > switch (pci_protocol_version) { > case PCI_PROTOCOL_VERSION_1_1: > size = hv_compose_msi_req_v1(&ctxt.int_pkts.v1, > - irq_data_get_affinity_mask(data), > + dest, > hpdev->desc.win_slot.slot, > cfg->vector); > break; > > case PCI_PROTOCOL_VERSION_1_2: > size = hv_compose_msi_req_v2(&ctxt.int_pkts.v2, > - irq_data_get_affinity_mask(data), > + dest, > hpdev->desc.win_slot.slot, > cfg->vector); > break; > -- > 2.7.4 Signed-off-by: Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] PCI: hv: use effective affinity mask
> -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: Tuesday, November 7, 2017 4:15 PM > To: Jake Oshins > Cc: Dexuan Cui ; Bjorn Helgaas > ; linux-...@vger.kernel.org; KY Srinivasan > ; Stephen Hemminger ; > de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; Haiyang Zhang > ; Jork Loeser ; > Chris Valean (Cloudbase Solutions SRL) ; Adrian > Suhov (Cloudbase Solutions SRL) ; Simon Xiao > ; 'Eyal Mizrachi' ; Jack > Morgenstein ; Armen Guezalian > ; Firas Mahameed ; > Tziporet Koren ; Daniel Jurgens > > Subject: Re: [PATCH] PCI: hv: use effective affinity mask > > On Wed, Nov 01, 2017 at 08:52:56PM +, Jake Oshins wrote: > > > -Original Message- > > > From: Dexuan Cui > > > Sent: Wednesday, November 1, 2017 1:31 PM > > > To: Bjorn Helgaas ; linux-...@vger.kernel.org; > > > Jake Oshins ; KY Srinivasan > > > ; Stephen Hemminger > > > > Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; > > > Haiyang Zhang ; Jork Loeser > > > ; Chris Valean (Cloudbase Solutions SRL) > > > ; Adrian Suhov (Cloudbase Solutions SRL) > > > ; Simon Xiao ; 'Eyal > > > Mizrachi' ; Jack Morgenstein > > > ; Armen Guezalian ; > Firas > > > Mahameed ; Tziporet Koren > > > ; Daniel Jurgens > > > Subject: [PATCH] PCI: hv: use effective affinity mask > > > > > > > > > The effective_affinity_mask is always set when an interrupt is > > > assigned in > > > __assign_irq_vector() -> apic->cpu_mask_to_apicid(), e.g. for struct > > > apic > > > apic_physflat: -> default_cpu_mask_to_apicid() -> > > > irq_data_update_effective_affinity(), but it looks > > > d->common->affinity remains all-1's before the user space or the kernel > changes it later. > > > > > > In the early allocation/initialization phase of an irq, we should > > > use the effective_affinity_mask, otherwise Hyper-V may not deliver > > > the interrupt to the expected cpu. Without the patch, if we assign 7 > > > Mellanox ConnectX-3 VFs to a 32-vCPU VM, one of the VFs may fail to > receive interrupts. > > > > > > Signed-off-by: Dexuan Cui > > > Cc: Jake Oshins > > > Cc: Jork Loeser > > > Cc: Stephen Hemminger > > > Cc: K. Y. Srinivasan > > > --- > > > > > > Please consider this for v4.14, if it's not too late. > > > > > > drivers/pci/host/pci-hyperv.c | 8 +--- > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/pci/host/pci-hyperv.c > > > b/drivers/pci/host/pci-hyperv.c index 5ccb47d..8b5f66d 100644 > > > --- a/drivers/pci/host/pci-hyperv.c > > > +++ b/drivers/pci/host/pci-hyperv.c > > > @@ -879,7 +879,7 @@ static void hv_irq_unmask(struct irq_data *data) > > > int cpu; > > > u64 res; > > > > > > - dest = irq_data_get_affinity_mask(data); > > > + dest = irq_data_get_effective_affinity_mask(data); > > > pdev = msi_desc_to_pci_dev(msi_desc); > > > pbus = pdev->bus; > > > hbus = container_of(pbus->sysdata, struct hv_pcibus_device, > > > sysdata); @@ -1042,6 +1042,7 @@ static void > > > hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg) > > > struct hv_pci_dev *hpdev; > > > struct pci_bus *pbus; > > > struct pci_dev *pdev; > > > + struct cpumask *dest; > > > struct compose_comp_ctxt comp; > > > struct tran_int_desc *int_desc; > > > struct { > > > @@ -1056,6 +1057,7 @@ static void hv_compose_msi_msg(struct > irq_data > > > *data, struct msi_msg *msg) > > > int ret; > > > > > > pdev = msi_desc_to_pci_dev(irq_data_get_msi_desc(data)); > > > + dest = irq_data_get_effective_affinity_mask(data); > > > pbus = pdev->bus; > > > hbus = container_of(pbus->sysdata, struct hv_pcibus_device, > > > sysdata); > > > hpdev = get_pcichild_wslot(hbus, devfn_to_wslot(pdev->devfn)); > @@ > > > -1081,14 +1083,14 @@ static void hv_compose_msi_msg(struct irq_data > > > *data, struct msi_msg *msg) > > > switch (pci_protocol_version) { > > > case PCI_PROTOCOL_VERSION_1_1: > > > size = hv_compose_msi_req_v1(&ctxt.int_pkts.v1, > > > - irq_data_get_affinity_mask(data), > > > + dest, > > >
[PATCH 1/1] staging: fix bcm/hostmibs.c checkpatch problems
From: Jake Edge Fix 4 checkpatch errors, many warnings in bcm/hostmibs.c Against next-20140321 tree Signed-off-by: Jake Edge --- There were two > 80 lines that I couldn't find a sensible way to split up, otherwise it is checkpatch clean diff --git a/drivers/staging/bcm/hostmibs.c b/drivers/staging/bcm/hostmibs.c index 39ace55..da7671d 100644 --- a/drivers/staging/bcm/hostmibs.c +++ b/drivers/staging/bcm/hostmibs.c @@ -9,27 +9,32 @@ #include "headers.h" -INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_mibs *pstHostMibs) +INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, + struct bcm_host_stats_mibs *pstHostMibs) { struct bcm_phs_entry *pstServiceFlowEntry = NULL; struct bcm_phs_rule *pstPhsRule = NULL; struct bcm_phs_classifier_table *pstClassifierTable = NULL; struct bcm_phs_classifier_entry *pstClassifierRule = NULL; - struct bcm_phs_extension *pDeviceExtension = (struct bcm_phs_extension *) &Adapter->stBCMPhsContext; + struct bcm_phs_extension *pDeviceExtension = + (struct bcm_phs_extension *) &Adapter->stBCMPhsContext; - UINT nClassifierIndex = 0, nPhsTableIndex = 0, nSfIndex = 0, uiIndex = 0; + UINT nClassifierIndex = 0, nPhsTableIndex = 0, + nSfIndex = 0, uiIndex = 0; if (pDeviceExtension == NULL) { - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, HOST_MIBS, DBG_LVL_ALL, "Invalid Device Extension\n"); + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, HOST_MIBS, + DBG_LVL_ALL, "Invalid Device Extension\n"); return STATUS_FAILURE; } /* Copy the classifier Table */ - for (nClassifierIndex = 0; nClassifierIndex < MAX_CLASSIFIERS; nClassifierIndex++) { + for (nClassifierIndex = 0; nClassifierIndex < MAX_CLASSIFIERS; + nClassifierIndex++) { if (Adapter->astClassifierTable[nClassifierIndex].bUsed == TRUE) - memcpy((PVOID) &pstHostMibs-> + memcpy((PVOID)&pstHostMibs-> astClassifierTable[nClassifierIndex], - (PVOID) &Adapter-> + (PVOID)&Adapter-> astClassifierTable[nClassifierIndex], sizeof(struct bcm_mibs_classifier_rule)); } @@ -37,8 +42,8 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_m /* Copy the SF Table */ for (nSfIndex = 0; nSfIndex < NO_OF_QUEUES; nSfIndex++) { if (Adapter->PackInfo[nSfIndex].bValid) { - memcpy((PVOID) &pstHostMibs->astSFtable[nSfIndex], - (PVOID) &Adapter->PackInfo[nSfIndex], + memcpy((PVOID)&pstHostMibs->astSFtable[nSfIndex], + (PVOID)&Adapter->PackInfo[nSfIndex], sizeof(struct bcm_mibs_table)); } else { /* If index in not valid, @@ -70,7 +75,8 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_m memcpy(&pstHostMibs-> astPhsRulesTable[nPhsTableIndex].u8PHSI, - &pstPhsRule->u8PHSI, sizeof(struct bcm_phs_rule)); + &pstPhsRule->u8PHSI, + sizeof(struct bcm_phs_rule)); nPhsTableIndex++; } @@ -82,53 +88,70 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_m /* Copy other Host Statistics parameters */ pstHostMibs->stHostInfo.GoodTransmits = Adapter->dev->stats.tx_packets; pstHostMibs->stHostInfo.GoodReceives = Adapter->dev->stats.rx_packets; - pstHostMibs->stHostInfo.CurrNumFreeDesc = atomic_read(&Adapter->CurrNumFreeTxDesc); + pstHostMibs->stHostInfo.CurrNumFreeDesc = + atomic_read(&Adapter->CurrNumFreeTxDesc); pstHostMibs->stHostInfo.BEBucketSize = Adapter->BEBucketSize; pstHostMibs->stHostInfo.rtPSBucketSize = Adapter->rtPSBucketSize; pstHostMibs->stHostInfo.TimerActive = Adapter->TimerActive; pstHostMibs->stHostInfo.u32TotalDSD = Adapter->u32TotalDSD; - memcpy(pstHostMibs->stHostInfo.aTxPktSizeHist, Adapter->aTxPktSizeHist, sizeof(UINT32) * MIBS_MAX_HIST_ENTRIES); - memcpy(pstHostMibs->stHostInfo.aRxPktSizeHist, Adapter->aRxPktSizeHist, si
Re: [PATCH 1/1] staging: fix bcm/hostmibs.c checkpatch problems
On Sun, 23 Mar 2014 10:45:09 -0700 Joe Perches wrote: > On Sat, 2014-03-22 at 09:50 -0600, Jake Edge wrote: > > Fix 4 checkpatch errors, many warnings in bcm/hostmibs.c > > Making code checkpatch clean shouldn't be the primary goal here. Perhaps not, but it was *my* goal :) > Removing uses of Hungarian-style notation, CamelCase naming, and > long variable names would be more helpful overall. I don't have enough experience to feel comfortable about doing that yet, but I did take some of your suggestions and am respinning the patch ... thanks, jake -- Jake Edge - j...@edge2.net - http://www.edge2.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/1] staging/bcm: fix hostmibs.c checkpatch problems
From: Jake Edge Fix 4 checkpatch errors, many warnings in bcm/hostmibs.c Against next-20140328 tree Signed-off-by: Jake Edge --- There were two > 80 lines that I couldn't find a sensible way to split up, otherwise it is checkpatch clean v2: fixes suggested by Joe Perches diff --git a/drivers/staging/bcm/hostmibs.c b/drivers/staging/bcm/hostmibs.c index 39ace55..d3b9adb 100644 --- a/drivers/staging/bcm/hostmibs.c +++ b/drivers/staging/bcm/hostmibs.c @@ -9,37 +9,42 @@ #include "headers.h" -INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_mibs *pstHostMibs) +INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, + struct bcm_host_stats_mibs *pstHostMibs) { struct bcm_phs_entry *pstServiceFlowEntry = NULL; struct bcm_phs_rule *pstPhsRule = NULL; struct bcm_phs_classifier_table *pstClassifierTable = NULL; struct bcm_phs_classifier_entry *pstClassifierRule = NULL; - struct bcm_phs_extension *pDeviceExtension = (struct bcm_phs_extension *) &Adapter->stBCMPhsContext; + struct bcm_phs_extension *pDeviceExtension = &Adapter->stBCMPhsContext; - UINT nClassifierIndex = 0, nPhsTableIndex = 0, nSfIndex = 0, uiIndex = 0; + UINT nClassifierIndex = 0, +nPhsTableIndex = 0, +nSfIndex = 0, +uiIndex = 0; if (pDeviceExtension == NULL) { - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, HOST_MIBS, DBG_LVL_ALL, "Invalid Device Extension\n"); + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, HOST_MIBS, + DBG_LVL_ALL, "Invalid Device Extension\n"); return STATUS_FAILURE; } /* Copy the classifier Table */ - for (nClassifierIndex = 0; nClassifierIndex < MAX_CLASSIFIERS; nClassifierIndex++) { + for (nClassifierIndex = 0; nClassifierIndex < MAX_CLASSIFIERS; + nClassifierIndex++) { if (Adapter->astClassifierTable[nClassifierIndex].bUsed == TRUE) - memcpy((PVOID) &pstHostMibs-> - astClassifierTable[nClassifierIndex], - (PVOID) &Adapter-> - astClassifierTable[nClassifierIndex], + memcpy(&pstHostMibs-> + astClassifierTable[nClassifierIndex], + &Adapter->astClassifierTable[nClassifierIndex], sizeof(struct bcm_mibs_classifier_rule)); } /* Copy the SF Table */ for (nSfIndex = 0; nSfIndex < NO_OF_QUEUES; nSfIndex++) { if (Adapter->PackInfo[nSfIndex].bValid) { - memcpy((PVOID) &pstHostMibs->astSFtable[nSfIndex], - (PVOID) &Adapter->PackInfo[nSfIndex], - sizeof(struct bcm_mibs_table)); + memcpy(&pstHostMibs->astSFtable[nSfIndex], + &Adapter->PackInfo[nSfIndex], + sizeof(struct bcm_mibs_table)); } else { /* If index in not valid, * don't process this for the PHS table. @@ -70,7 +75,8 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_m memcpy(&pstHostMibs-> astPhsRulesTable[nPhsTableIndex].u8PHSI, - &pstPhsRule->u8PHSI, sizeof(struct bcm_phs_rule)); + &pstPhsRule->u8PHSI, + sizeof(struct bcm_phs_rule)); nPhsTableIndex++; } @@ -82,53 +88,70 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_m /* Copy other Host Statistics parameters */ pstHostMibs->stHostInfo.GoodTransmits = Adapter->dev->stats.tx_packets; pstHostMibs->stHostInfo.GoodReceives = Adapter->dev->stats.rx_packets; - pstHostMibs->stHostInfo.CurrNumFreeDesc = atomic_read(&Adapter->CurrNumFreeTxDesc); + pstHostMibs->stHostInfo.CurrNumFreeDesc = + atomic_read(&Adapter->CurrNumFreeTxDesc); pstHostMibs->stHostInfo.BEBucketSize = Adapter->BEBucketSize; pstHostMibs->stHostInfo.rtPSBucketSize = Adapter->rtPSBucketSize; pstHostMibs->stHostInfo.TimerActive = Adapter->TimerActive; pstHostMibs->stHostInfo.u32TotalDSD = Adapter->u32TotalDSD; - memcpy(pstHostMibs->stHostInfo.aTxPktSizeHist, Adapter->aTxPktSizeHist, sizeof(UINT32) * MIBS_MA
[PATCHv3 1/1] staging/bcm fix hostmibs.c checkpatch problems
Fix 4 checkpatch errors, many warnings in bcm/hostmibs.c Signed-off-by: Jake Edge --- Against next-20140403 v3: fixes suggested by Dan Carpenter v2: fixes suggested by Joe Perches There are still some > 80 length lines that require more substantive rework to fix. drivers/staging/bcm/hostmibs.c | 51 +- 1 file changed, 30 insertions(+), 21 deletions(-) diff --git a/drivers/staging/bcm/hostmibs.c b/drivers/staging/bcm/hostmibs.c index 39ace55..42d9004 100644 --- a/drivers/staging/bcm/hostmibs.c +++ b/drivers/staging/bcm/hostmibs.c @@ -9,37 +9,40 @@ #include "headers.h" -INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_mibs *pstHostMibs) +INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, + struct bcm_host_stats_mibs *pstHostMibs) { struct bcm_phs_entry *pstServiceFlowEntry = NULL; struct bcm_phs_rule *pstPhsRule = NULL; struct bcm_phs_classifier_table *pstClassifierTable = NULL; struct bcm_phs_classifier_entry *pstClassifierRule = NULL; - struct bcm_phs_extension *pDeviceExtension = (struct bcm_phs_extension *) &Adapter->stBCMPhsContext; - - UINT nClassifierIndex = 0, nPhsTableIndex = 0, nSfIndex = 0, uiIndex = 0; + struct bcm_phs_extension *pDeviceExtension = &Adapter->stBCMPhsContext; + UINT nClassifierIndex = 0; + UINT nPhsTableIndex = 0; + UINT nSfIndex = 0; + UINT uiIndex = 0; if (pDeviceExtension == NULL) { - BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, HOST_MIBS, DBG_LVL_ALL, "Invalid Device Extension\n"); + BCM_DEBUG_PRINT(Adapter, DBG_TYPE_OTHERS, HOST_MIBS, + DBG_LVL_ALL, "Invalid Device Extension\n"); return STATUS_FAILURE; } /* Copy the classifier Table */ - for (nClassifierIndex = 0; nClassifierIndex < MAX_CLASSIFIERS; nClassifierIndex++) { + for (nClassifierIndex = 0; nClassifierIndex < MAX_CLASSIFIERS; + nClassifierIndex++) { if (Adapter->astClassifierTable[nClassifierIndex].bUsed == TRUE) - memcpy((PVOID) &pstHostMibs-> - astClassifierTable[nClassifierIndex], - (PVOID) &Adapter-> - astClassifierTable[nClassifierIndex], + memcpy(&pstHostMibs->astClassifierTable[nClassifierIndex], + &Adapter->astClassifierTable[nClassifierIndex], sizeof(struct bcm_mibs_classifier_rule)); } /* Copy the SF Table */ for (nSfIndex = 0; nSfIndex < NO_OF_QUEUES; nSfIndex++) { if (Adapter->PackInfo[nSfIndex].bValid) { - memcpy((PVOID) &pstHostMibs->astSFtable[nSfIndex], - (PVOID) &Adapter->PackInfo[nSfIndex], - sizeof(struct bcm_mibs_table)); + memcpy(&pstHostMibs->astSFtable[nSfIndex], + &Adapter->PackInfo[nSfIndex], + sizeof(struct bcm_mibs_table)); } else { /* If index in not valid, * don't process this for the PHS table. @@ -68,9 +71,9 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_m pstHostMibs->astPhsRulesTable[nPhsTableIndex]. ulSFID = Adapter->PackInfo[nSfIndex].ulSFID; - memcpy(&pstHostMibs-> - astPhsRulesTable[nPhsTableIndex].u8PHSI, - &pstPhsRule->u8PHSI, sizeof(struct bcm_phs_rule)); + memcpy(&pstHostMibs->astPhsRulesTable[nPhsTableIndex].u8PHSI, + &pstPhsRule->u8PHSI, + sizeof(struct bcm_phs_rule)); nPhsTableIndex++; } @@ -82,26 +85,32 @@ INT ProcessGetHostMibs(struct bcm_mini_adapter *Adapter, struct bcm_host_stats_m /* Copy other Host Statistics parameters */ pstHostMibs->stHostInfo.GoodTransmits = Adapter->dev->stats.tx_packets; pstHostMibs->stHostInfo.GoodReceives = Adapter->dev->stats.rx_packets; - pstHostMibs->stHostInfo.CurrNumFreeDesc = atomic_read(&Adapter->CurrNumFreeTxDesc); + pstHostMibs->stHostInfo.CurrNumFreeDesc = + atomic_read(&Adapter->CurrNumFreeTxDesc); pstHostMibs->stHostInfo.BEBucketSize = Adapter->BEBucketSize;
[PATCH] staging/rtl8187se fix sparse complaints
Fix the following sparse complaints: drivers/staging/rtl8187se//r8180_wx.c:1399:24: warning: symbol 'r8180_wx_handlers_def' was not declared. Should it be static? drivers/staging/rtl8187se//ieee80211/ieee80211_crypt.c:201:5: warning: symbol 'ieee80211_crypto_init' was not declared. Should it be static? drivers/staging/rtl8187se//ieee80211/ieee80211_crypt.c:222:6: warning: symbol 'ieee80211_crypto_deinit' was not declared. Should it be static? drivers/staging/rtl8187se//ieee80211/ieee80211_crypt_ccmp.c:446:5: warning: symbol 'ieee80211_crypto_ccmp_init' was not declared. Should it be static? drivers/staging/rtl8187se//ieee80211/ieee80211_crypt_ccmp.c:452:6: warning: symbol 'ieee80211_crypto_ccmp_exit' was not declared. Should it be static? drivers/staging/rtl8187se//ieee80211/ieee80211_crypt_wep.c:264:5: warning: symbol 'ieee80211_crypto_wep_init' was not declared. Should it be static? drivers/staging/rtl8187se//ieee80211/ieee80211_crypt_wep.c:269:6: warning: symbol 'ieee80211_crypto_wep_exit' was not declared. Should it be static? by adding an include file into source files and moving some declarations around into the proper header files. Signed-off-by: Jake Edge --- Against next-20140411 diff --git a/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.h b/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.h index 0b4ea43..8fe7873 100644 --- a/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.h +++ b/drivers/staging/rtl8187se/ieee80211/ieee80211_crypt.h @@ -83,4 +83,13 @@ void ieee80211_crypt_deinit_handler(unsigned long); void ieee80211_crypt_delayed_deinit(struct ieee80211_device *ieee, struct ieee80211_crypt_data **crypt); +/* fun with the built-in ieee80211 stack... */ +int ieee80211_crypto_init(void); +void ieee80211_crypto_deinit(void); +int ieee80211_crypto_tkip_init(void); +void ieee80211_crypto_tkip_exit(void); +int ieee80211_crypto_ccmp_init(void); +void ieee80211_crypto_ccmp_exit(void); +int ieee80211_crypto_wep_init(void); +void ieee80211_crypto_wep_exit(void); #endif diff --git a/drivers/staging/rtl8187se/r8180.h b/drivers/staging/rtl8187se/r8180.h index 9f931db..cf510b6 100644 --- a/drivers/staging/rtl8187se/r8180.h +++ b/drivers/staging/rtl8187se/r8180.h @@ -629,12 +629,3 @@ bool MgntActSet_RF_State(struct net_device *dev, enum rt_rf_power_state StateToS #endif -/* fun with the built-in ieee80211 stack... */ -extern int ieee80211_crypto_init(void); -extern void ieee80211_crypto_deinit(void); -extern int ieee80211_crypto_tkip_init(void); -extern void ieee80211_crypto_tkip_exit(void); -extern int ieee80211_crypto_ccmp_init(void); -extern void ieee80211_crypto_ccmp_exit(void); -extern int ieee80211_crypto_wep_init(void); -extern void ieee80211_crypto_wep_exit(void); diff --git a/drivers/staging/rtl8187se/r8180_core.c b/drivers/staging/rtl8187se/r8180_core.c index a6022d4..cbdff00 100644 --- a/drivers/staging/rtl8187se/r8180_core.c +++ b/drivers/staging/rtl8187se/r8180_core.c @@ -47,6 +47,7 @@ #include "r8180_dm.h" #include "ieee80211/dot11d.h" +#include "ieee80211/ieee80211_crypt.h" static struct pci_device_id rtl8180_pci_id_tbl[] = { { diff --git a/drivers/staging/rtl8187se/r8180_wx.c b/drivers/staging/rtl8187se/r8180_wx.c index b552491..57bf01f 100644 --- a/drivers/staging/rtl8187se/r8180_wx.c +++ b/drivers/staging/rtl8187se/r8180_wx.c @@ -20,6 +20,7 @@ #include "r8180.h" #include "r8180_hw.h" +#include "r8180_wx.h" #include #include "ieee80211/dot11d.h" -- Jake Edge - j...@edge2.net - http://www.edge2.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v6 0/7] PCI: hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: ja...@microsoft.com [mailto:ja...@microsoft.com] > Sent: Monday, November 2, 2015 1:33 PM > To: gre...@linuxfoundation.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@redhat.com; t...@redhat.com; Haiyang > Zhang ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Cc: Jake Oshins > Subject: [PATCH v6 0/7] PCI: hv: New paravirtual PCI front-end for Hyper-V > VMs > > From: Jake Oshins > > This version of this patch series incorporates feedback from Andy > Shevchenko. > > First, export functions that allow correlating Hyper-V virtual processors > and Linux cpus, along with the means for invoking a hypercall that targets > interrupts at chosen vectors on specfic cpus. > > Second, mark various parts of IRQ domain related code as exported, so that > this PCI front-end can implement an IRQ domain as part of a module. (The > alternative would be to pull all this into the kernel, which would pull > in a lot of other Hyper-V related code, as this IRQ domain depends on > hv_vmbus.ko.) > > Third, modify PCI so that new root PCI buses can be marked with an > associated > fwnode_handle, and so that root PCI buses can look up their associated IRQ > domain by that handle. > > Fourth, introduce a new driver, hv_pcifront, which eposes root PCI buses in > a Hyper-V VM. These root PCI buses expose real PCIe devices, or PCI Virtual > Functions. > > Jake Oshins (7): > drivers:hv: Export a function that maps Linux CPU num onto Hyper-V > proc num > drivers:hv: Export hv_do_hypercall() > PCI: Make it possible to implement a PCI MSI IRQ Domain in a module. > PCI: Add fwnode_handle to pci_sysdata > PCI: irqdomain: Look up IRQ domain by fwnode_handle > drivers:hv: Define the channel type of Hyper-V PCI Express > pass-through > PCI: hv: New paravirtual PCI front-end for Hyper-V VMs > > MAINTAINERS|1 + > arch/x86/include/asm/msi.h |4 + > arch/x86/include/asm/pci.h | 13 + > arch/x86/kernel/apic/msi.c |5 +- > arch/x86/kernel/apic/vector.c |2 + > drivers/hv/hv.c| 20 +- > drivers/hv/hyperv_vmbus.h |2 +- > drivers/hv/vmbus_drv.c | 17 + > drivers/pci/Kconfig|7 + > drivers/pci/host/Makefile |1 + > drivers/pci/host/hv_pcifront.c | 2267 > > drivers/pci/msi.c |4 + > drivers/pci/probe.c| 14 + > include/asm-generic/pci.h |4 + > include/linux/hyperv.h | 14 + > kernel/irq/chip.c |1 + > kernel/irq/irqdomain.c |2 + > 17 files changed, 2365 insertions(+), 13 deletions(-) > create mode 100644 drivers/pci/host/hv_pcifront.c > > -- > 1.9.1 Apparently I've been using the wrong e-mail address for Thomas Gleixner. Sorry about that. Are there any more comments about this patch series? Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v6 4/7] PCI: Add fwnode_handle to pci_sysdata
> -Original Message- > From: Jiang Liu [mailto:jiang@linux.intel.com] > Sent: Wednesday, December 2, 2015 10:53 PM > To: Jake Oshins ; gre...@linuxfoundation.org; KY > Srinivasan ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; t...@redhat.com; Haiyang Zhang > ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Subject: Re: [PATCH v6 4/7] PCI: Add fwnode_handle to pci_sysdata > > On 2015/11/3 5:33, ja...@microsoft.com wrote: > > From: Jake Oshins > > > > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > > +static inline void *_pci_root_bus_fwnode(struct pci_bus *bus) > > +{ > > + struct pci_sysdata *sd = bus->sysdata; > > + return sd->fwnode; > > +} > > + > > +#define pci_root_bus_fwnode_pci_root_bus_fwnode > > +#endif > > + > > /* Can be used to override the logic in pci_scan_bus for skipping > > already-configured bus numbers - to be used for buggy BIOSes > > or architectures with incomplete PCI setup by the loader */ > > diff --git a/include/asm-generic/pci.h b/include/asm-generic/pci.h > > index f24bc51..3fde985 100644 > > --- a/include/asm-generic/pci.h > > +++ b/include/asm-generic/pci.h > > @@ -21,4 +21,8 @@ static inline int pci_get_legacy_ide_irq(struct pci_dev > *dev, int channel) > > #define PCI_DMA_BUS_IS_PHYS(1) > > #endif > > > > +#ifndef pci_root_bus_fwnode > > +#define pci_root_bus_fwnode(bus) ((void)(bus),NULL) > > +#endif > Hi Jakeo, > For x86, all PCI devices share the same MSI controller. But I'm > not sure whether it may have per-bus/per-device MSI controllers on other > archs. If there may be multiple MSI controllers serving PCI devices > under the same PCI root, it would be better to use some thing like > pci_get_msi_fwnode(bus) or similar. > Thanks, > Gerry > Certainly other architectures have per-bus MSI controllers, though usually it's per root complex. In case you're not familiar with PCI Express terms (and I apologize for this if you are) a root complex is one instance of PCI Express, where traffic is defined between every part of it. If your traffic leaves PCI Express and goes onto some processor-specific bus, it has left the root complex. The root complex is usually modeled as a root PCI bus, a set of root port, and a set of legacy and/or embedded endpoints, which amounts to saying that some PCI/E devices are built into the root complex and not connected with links. I don't have a use case at the moment for anything other than an MSI controller for a root complex. If you'd like me to change this so that it more naturally extends to a situation where you'd want to use it for a specific bus, I can do that. Please let me know. And thanks again for your review. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v6 7/7] PCI: hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: Jiang Liu [mailto:jiang@linux.intel.com] > Sent: Wednesday, December 2, 2015 7:12 PM > To: Jake Oshins ; gre...@linuxfoundation.org; KY > Srinivasan ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; t...@redhat.com; Haiyang Zhang > ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Subject: Re: [PATCH v6 7/7] PCI: hv: New paravirtual PCI front-end for Hyper- > V VMs > > On 2015/11/3 5:33, ja...@microsoft.com wrote: > > From: Jake Oshins > > [...] > > + > > +/** > > + * hv_irq_unmask() - "Unmask" the IRQ by setting its current > > + * affinity. > > + * @data: Describes the IRQ > > + * > > + * Build new a destination for the MSI and make a hypercall to > > + * update the Interrupt Redirection Table. "Device Logical ID" > > + * is built out of this PCI bus's instance GUID and the function > > + * number of the device. > > + */ > > +void hv_irq_unmask(struct irq_data *data) > > +{ > > + struct msi_desc *msi_desc = irq_data_get_msi_desc(data); > > + struct irq_cfg *cfg = irqd_cfg(data); > > + struct retarget_msi_interrupt params; > > + struct hv_pcibus_device *hbus; > > + struct cpumask *dest; > > + struct pci_bus *pbus; > > + struct pci_dev *pdev; > > + int cpu; > > + > > + dest = irq_data_get_affinity_mask(data); > > + pdev = msi_desc_to_pci_dev(msi_desc); > > + pbus = pdev->bus; > > + hbus = container_of(pbus->sysdata, struct hv_pcibus_device, > sysdata); > > + > > + memset(¶ms, 0, sizeof(params)); > > + params.partition_id = HV_PARTITION_ID_SELF; > > + params.source = 1; /* MSI(-X) */ > > + params.address = msi_desc->msg.address_lo; > > + params.data = msi_desc->msg.data; > > + params.device_id = (hbus->hdev->dev_instance.b[5] << 24) | > > + (hbus->hdev->dev_instance.b[4] << 16) | > > + (hbus->hdev->dev_instance.b[7] << 8) | > > + (hbus->hdev->dev_instance.b[6] & 0xf8) | > > + PCI_FUNC(pdev->devfn); > > + params.vector = cfg->vector; > > + > > + for_each_cpu_and(cpu, dest, cpu_online_mask) > > + params.vp_mask |= (1 << > vmbus_cpu_number_to_vp_number(cpu)); > No knowledge about the HV implementation details, but feel some chances > of race here between hv_irq_unmask(), hv_set_affinity() and > cpu_up()/cpu_down() when accessing 'dest' and cpu_online_mask. > Thanks. Is there any architectural contract here? I tried implementing this by doing this work in the set_affinity() callback, but the vector was often wrong when that callback was invoked. (It seems to get changed just after set_affinity().) Can you suggest a durable strategy? I'll respond to all the other comments you sent (and this one, once I understand the right response) and resend. Thanks for your review, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v7 4/7] PCI: Add fwnode_handle to pci_sysdata
> -Original Message- > From: Marc Zyngier [mailto:marc.zyng...@arm.com] > Sent: Wednesday, December 9, 2015 8:51 AM > To: Jake Oshins ; gre...@linuxfoundation.org; KY > Srinivasan ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; t...@linutronix.de; Haiyang Zhang > ; bhelg...@google.com; linux- > p...@vger.kernel.org > Subject: Re: [PATCH v7 4/7] PCI: Add fwnode_handle to pci_sysdata > > On 05/12/15 00:36, ja...@microsoft.com wrote: > > From: Jake Oshins > > > > This patch adds an fwnode_handle to struct pci_sysdata, which is > > used by the next patch in the series when trying to locate an > > IRQ domain associated with a root PCI bus. > > > > Signed-off-by: Jake Oshins > > --- > > arch/x86/include/asm/pci.h | 15 +++ > > include/asm-generic/pci.h | 4 > > 2 files changed, 19 insertions(+) > > > > [...] > > > diff --git a/include/asm-generic/pci.h b/include/asm-generic/pci.h > > index f24bc51..4092886 100644 > > --- a/include/asm-generic/pci.h > > +++ b/include/asm-generic/pci.h > > @@ -21,4 +21,8 @@ static inline int pci_get_legacy_ide_irq(struct pci_dev > *dev, int channel) > > #define PCI_DMA_BUS_IS_PHYS(1) > > #endif > > > > +#ifndef pci_root_bus_fwnode > > +#define pci_root_bus_fwnode(bus) ((void)(bus), NULL) > > +#endif > > + > > #endif /* _ASM_GENERIC_PCI_H */ > > > > This breaks at least arm64 (as you can see from the reply to patch #5, > because it does have its own asm/pci.h. Instead, how about moving this > to linux/pci.h, just after the include of asm/pci.h? I just gave it a > go, and it seems to work nicely (the first hunk fixes the rest of the > arm64 compile issue): Thank you. I was just working through how to do that. I lost a couple of days trying to figure out how to cross-compile for arm64 to check that any fix that I made actually worked. (In the process, I've completely messed up my development machine. Any pointers to the right strategy would be appreciated.) -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v9 4/7] PCI: Add fwnode_handle to pci_sysdata
> -Original Message- > From: ja...@microsoft.com [mailto:ja...@microsoft.com] > Sent: Wednesday, December 9, 2015 2:55 PM > To: gre...@linuxfoundation.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@redhat.com; t...@linutronix.de; Haiyang > Zhang ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Cc: Jake Oshins > Subject: [PATCH v9 4/7] PCI: Add fwnode_handle to pci_sysdata > > From: Jake Oshins > > This patch adds an fwnode_handle to struct pci_sysdata, which is > used by the next patch in the series when trying to locate an > IRQ domain associated with a root PCI bus. > > Signed-off-by: Jake Oshins > --- > arch/x86/include/asm/pci.h | 15 +++ > drivers/pci/probe.c| 1 + > include/linux/pci.h| 4 > 3 files changed, 20 insertions(+) > > diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h > index 4625943..6fc3c7c 100644 > --- a/arch/x86/include/asm/pci.h > +++ b/arch/x86/include/asm/pci.h > @@ -20,6 +20,9 @@ struct pci_sysdata { > #ifdef CONFIG_X86_64 > void*iommu; /* IOMMU private data */ > #endif > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > + void*fwnode;/* IRQ domain for MSI assignment */ > +#endif > }; > > extern int pci_routeirq; > @@ -32,6 +35,7 @@ extern int noioapicreroute; > static inline int pci_domain_nr(struct pci_bus *bus) > { > struct pci_sysdata *sd = bus->sysdata; > + > return sd->domain; > } > > @@ -41,6 +45,17 @@ static inline int pci_proc_domain(struct pci_bus *bus) > } > #endif > > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > +static inline void *_pci_root_bus_fwnode(struct pci_bus *bus) > +{ > + struct pci_sysdata *sd = bus->sysdata; > + > + return sd->fwnode; > +} > + > +#define pci_root_bus_fwnode _pci_root_bus_fwnode > +#endif > + > /* Can be used to override the logic in pci_scan_bus for skipping > already-configured bus numbers - to be used for buggy BIOSes > or architectures with incomplete PCI setup by the loader */ > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index edb1984..750f907 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -15,6 +15,7 @@ > #include > #include > #include > +#include > #include > #include "pci.h" > > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 6ae25aa..b414422 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1517,6 +1517,10 @@ static inline int pci_get_new_domain_nr(void) { > return -ENOSYS; } > > #include > > +#ifndef pci_root_bus_fwnode > +#define pci_root_bus_fwnode(bus) ((void)(bus), NULL) > +#endif > + > /* these helpers provide future and backwards compatibility > * for accessing popular PCI BAR info */ > #define pci_resource_start(dev, bar) ((dev)->resource[(bar)].start) > -- > 1.9.1 Bjorn, are you willing to take this patch and the next one in the series through your tree? The other patches in this series have already been taken into linux-next by either Greg K-H or Thomas Gleixner, or they're dependent on these two. Thomas has suggested that the PCI tree would be the right one to incorporate them. Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/7] drivers:hv: Make a function to free mmio regions through vmbus
This patch introduces a function that reverses everything done by vmbus_allocate_mmio(). Existing code just called release_mem_region(). Future patches in this series require a more complex sequence of actions, so this function is introduced to wrap those actions. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 15 +++ include/linux/hyperv.h | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 799518b..60553c1 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1188,6 +1188,21 @@ exit: EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); /** + * vmbus_free_mmio() - Free a memory-mapped I/O range. + * @start: Base address of region to release. + * @size: Size of the range to be allocated + * + * This function releases anything requested by + * vmbus_mmio_allocate(). + */ +void vmbus_free_mmio(resource_size_t start, resource_size_t size) +{ + release_mem_region(start, size); + +} +EXPORT_SYMBOL_GPL(vmbus_free_mmio); + +/** * vmbus_cpu_number_to_vp_number() - Map CPU to VP. * @cpu_number: CPU number in Linux terms * diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index aa0fadc..ecd81c3 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1091,7 +1091,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t min, resource_size_t max, resource_size_t size, resource_size_t align, bool fb_overlap_ok); - +void vmbus_free_mmio(resource_size_t start, resource_size_t size); int vmbus_cpu_number_to_vp_number(int cpu_number); u64 hv_do_hypercall(u64 control, void *input, void *output); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/7] drivers:hv: Lock access to hyperv_mmio resource tree
In existing code, this tree of resources is created in single-threaded code and never modified after it is created, and thus needs no locking. This patch introduces a semaphore for tree access, as other patches in this series introduce run-time modifications of this resource tree which can happen on multiple threads. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 64713ff..799518b 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -102,6 +102,7 @@ static struct notifier_block hyperv_panic_block = { }; struct resource *hyperv_mmio; +DEFINE_SEMAPHORE(hyperv_mmio_lock); static int vmbus_exists(void) { @@ -1132,7 +1133,10 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t range_min, range_max, start, local_min, local_max; const char *dev_n = dev_name(&device_obj->device); u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); - int i; + int i, retval; + + retval = -ENXIO; + down(&hyperv_mmio_lock); for (iter = hyperv_mmio; iter; iter = iter->sibling) { if ((iter->start >= max) || (iter->end <= min)) @@ -1169,13 +1173,17 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, for (; start + size - 1 <= local_max; start += align) { *new = request_mem_region_exclusive(start, size, dev_n); - if (*new) - return 0; + if (*new) { + retval = 0; + goto exit; + } } } } - return -ENXIO; +exit: + up(&hyperv_mmio_lock); + return retval; } EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/7] drivers:hv: Ensure that bridge windows don't overlap
This series differs from v1 in that the last two patches have been modified to simplify the tracking of the frame buffer area during early boot, which is done so that other devices which may get probed and started first don't choose memory space that's in use by that frame buffer. Hyper-V VMs expose paravirtual drivers through a mechanism called VMBus, which is managed by hv_vmbus.ko. For each parvirtual service instance, this driver exposes a new child device. Some of these child devices need memory address space, into which Hyper-V will map things like the virtual video frame buffer. This memory-mapped address space is chosen by the guest OS, not the hypervisor. This is difficult to map onto the Linux pnp layer, as the code in the pnp layer to choose MMIO space keys off of bus type and it doesn't know anything about VMBus. The maintainers of the pnp layer have asked that we not offer patches to it that make it understand VMBus, but that we rather find ways of using the code in its current state. So hv_vmbus.ko exports a function, vmbus_allocate_mmio() for choosing the address space for any child driver that needs this facility. The recently introduced PCI front-end driver for Hyper-V VMs (pci-hyperv.ko) uses vmbus_allocate_mmio() for choosing both the region of memory space into which real PCI Express devices are mapped. The regions allocated are made to look like root PCI bus bridge windows to the PCI driver, reusing all the code in the PCI driver for the rest of PCI device management. The problem is that these bridge windows are marked in such a way that devices can still allocate from the memory space spanned by them, and this means that if two different PCI buses are created in the VM, each with devices under them, they may allocate the same memory space, leading to PCI Base Address Register which overlap. This patch series fixes the problem by tracking allocations to child devices in a separate resource tree, marking them such that the bridge windows can't overlap. The main memory resource tree, iomem_resource, contains resources properly marked as bridge windows, allowing their children to overlap with them. Jake Oshins (7): drivers:hv: Lock access to hyperv_mmio resource tree drivers:hv: Make a function to free mmio regions through vmbus drivers:hv: Use new vmbus_mmio_free() from client drivers. drivers:hv: Reverse order of resources in hyperv_mmio drivers:hv: Track allocations of children of hv_vmbus in private resource tree drivers:hv: Record MMIO range in use by frame buffer drivers:hv: Separate out frame buffer logic when picking MMIO range drivers/hv/vmbus_drv.c | 142 +--- drivers/pci/host/pci-hyperv.c | 14 ++-- drivers/video/fbdev/hyperv_fb.c | 4 +- include/linux/hyperv.h | 2 +- 4 files changed, 114 insertions(+), 48 deletions(-) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.
This patch modifies all the callers of vmbus_mmio_allocate() to call vmbus_mmio_free() instead of release_mem_region(). Signed-off-by: Jake Oshins --- drivers/pci/host/pci-hyperv.c | 14 +++--- drivers/video/fbdev/hyperv_fb.c | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index ed651ba..f2559b6 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -1795,14 +1795,14 @@ static void hv_pci_free_bridge_windows(struct hv_pcibus_device *hbus) if (hbus->low_mmio_space && hbus->low_mmio_res) { hbus->low_mmio_res->flags |= IORESOURCE_BUSY; - release_mem_region(hbus->low_mmio_res->start, - resource_size(hbus->low_mmio_res)); + vmbus_free_mmio(hbus->low_mmio_res->start, + resource_size(hbus->low_mmio_res)); } if (hbus->high_mmio_space && hbus->high_mmio_res) { hbus->high_mmio_res->flags |= IORESOURCE_BUSY; - release_mem_region(hbus->high_mmio_res->start, - resource_size(hbus->high_mmio_res)); + vmbus_free_mmio(hbus->high_mmio_res->start, + resource_size(hbus->high_mmio_res)); } } @@ -1880,8 +1880,8 @@ static int hv_pci_allocate_bridge_windows(struct hv_pcibus_device *hbus) release_low_mmio: if (hbus->low_mmio_res) { - release_mem_region(hbus->low_mmio_res->start, - resource_size(hbus->low_mmio_res)); + vmbus_free_mmio(hbus->low_mmio_res->start, + resource_size(hbus->low_mmio_res)); } return ret; @@ -1924,7 +1924,7 @@ static int hv_allocate_config_window(struct hv_pcibus_device *hbus) static void hv_free_config_window(struct hv_pcibus_device *hbus) { - release_mem_region(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH); + vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH); } /** diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index e2451bd..2fd49b2 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -743,7 +743,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) err3: iounmap(fb_virt); err2: - release_mem_region(par->mem->start, screen_fb_size); + vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL; err1: if (!gen2vm) @@ -758,7 +758,7 @@ static void hvfb_putmem(struct fb_info *info) struct hvfb_par *par = info->par; iounmap(info->screen_base); - release_mem_region(par->mem->start, screen_fb_size); + vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL; } -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 6/7] drivers:hv: Record MMIO range in use by frame buffer
Later in the boot sequence, we need to figure out which memory ranges can be given out to various paravirtual drivers. The hyperv_fb driver should, ideally, be placed right on top of the frame buffer, without some other device getting plopped on top of this range in the meantime. Recording this now allows that to be guaranteed by the code introduced in the next patch. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 37 - 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index dfc6149..f8fa7b8 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -41,6 +41,7 @@ #include #include #include +#include #include "hyperv_vmbus.h" static struct acpi_device *hv_acpi_dev; @@ -101,6 +102,8 @@ static struct notifier_block hyperv_panic_block = { .notifier_call = hyperv_panic_event, }; +static const char *fb_mmio_name = "fb_range"; +static struct resource *fb_mmio; struct resource *hyperv_mmio; DEFINE_SEMAPHORE(hyperv_mmio_lock); @@ -1091,6 +1094,12 @@ static int vmbus_acpi_remove(struct acpi_device *device) struct resource *next_res; if (hyperv_mmio) { + if (fb_mmio) { + __release_region(hyperv_mmio, fb_mmio->start, +fb_mmio->end - fb_mmio->start + 1); + fb_mmio = NULL; + } + for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) { next_res = cur_res->sibling; kfree(cur_res); @@ -1100,6 +1109,30 @@ static int vmbus_acpi_remove(struct acpi_device *device) return 0; } +static void vmbus_reserve_fb(void) +{ + int size; + /* +* Make a claim for the frame buffer in the resource tree under the +* first node, which will be the one below 4GB. The length seems to +* be underreported, particularly in a Generation 1 VM. So start out +* reserving a larger area and make it smaller until it succeeds. +*/ + + if (screen_info.lfb_base) { + if (efi_enabled(EFI_BOOT)) + size = screen_info.lfb_size; + else + size = max_t(__u32, screen_info.lfb_size, 0x400); + + for (; !fb_mmio && (size >= 0x10); size >>= 1) { + fb_mmio = __request_region(hyperv_mmio, + screen_info.lfb_base, size, + fb_mmio_name, 0); + } + } +} + /** * vmbus_allocate_mmio() - Pick a memory-mapped I/O range. * @new: If successful, supplied a pointer to the @@ -1261,8 +1294,10 @@ static int vmbus_acpi_add(struct acpi_device *device) if (ACPI_FAILURE(result)) continue; - if (hyperv_mmio) + if (hyperv_mmio) { + vmbus_reserve_fb(); break; + } } ret_val = 0; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 4/7] drivers:hv: Reverse order of resources in hyperv_mmio
A patch later in this series allocates child nodes in this resource tree. For that to work, this tree needs to be sorted in ascending order. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 60553c1..1ce47d0 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1049,7 +1049,6 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) new_res->end = end; /* -* Stick ranges from higher in address space at the front of the list. * If two ranges are adjacent, merge them. */ do { @@ -1070,7 +1069,7 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) break; } - if ((*old_res)->end < new_res->start) { + if ((*old_res)->start > new_res->end) { new_res->sibling = *old_res; if (prev_res) (*prev_res)->sibling = new_res; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 7/7] drivers:hv: Separate out frame buffer logic when picking MMIO range
Simplify the logic that picks MMIO ranges by pulling out the logic related to trying to lay frame buffer claim on top of where the firmware placed the frame buffer. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 79 ++ 1 file changed, 34 insertions(+), 45 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index f8fa7b8..ec6fa42 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1162,64 +1162,53 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, bool fb_overlap_ok) { struct resource *iter, *shadow; - resource_size_t range_min, range_max, start, local_min, local_max; + resource_size_t range_min, range_max, start; const char *dev_n = dev_name(&device_obj->device); - u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); - int i, retval; + int retval; retval = -ENXIO; down(&hyperv_mmio_lock); + /* +* If overlaps with frame buffers are allowed, then first attempt to +* make the allocation from within the reserved region. Because it +* is already reserved, no shadow allocation is necessary. +*/ + if (fb_overlap_ok && fb_mmio) { + + range_min = fb_mmio->start; + range_max = fb_mmio->end; + start = (range_min + align - 1) & ~(align - 1); + for (; start + size - 1 <= range_max; start += align) { + *new = request_mem_region_exclusive(start, size, dev_n); + if (*new) { + retval = 0; + goto exit; + } + } + } + for (iter = hyperv_mmio; iter; iter = iter->sibling) { if ((iter->start >= max) || (iter->end <= min)) continue; range_min = iter->start; range_max = iter->end; - - /* If this range overlaps the frame buffer, split it into - two tries. */ - for (i = 0; i < 2; i++) { - local_min = range_min; - local_max = range_max; - if (fb_overlap_ok || (range_min >= fb_end) || - (range_max <= screen_info.lfb_base)) { - i++; - } else { - if ((range_min <= screen_info.lfb_base) && - (range_max >= screen_info.lfb_base)) { - /* -* The frame buffer is in this window, -* so trim this into the part that -* preceeds the frame buffer. -*/ - local_max = screen_info.lfb_base - 1; - range_min = fb_end; - } else { - range_min = fb_end; - continue; - } + start = (range_min + align - 1) & ~(align - 1); + for (; start + size - 1 <= range_max; start += align) { + shadow = __request_region(iter, start, size, NULL, + IORESOURCE_BUSY); + if (!shadow) + continue; + + *new = request_mem_region_exclusive(start, size, dev_n); + if (*new) { + shadow->name = (char *)*new; + retval = 0; + goto exit; } - start = (local_min + align - 1) & ~(align - 1); - for (; start + size - 1 <= local_max; start += align) { - shadow = __request_region(iter, start, - size, - NULL, - IORESOURCE_BUSY); - if (!shadow) - continue; - - *new = request_mem_region_exclusive(start, size, - dev_n); - if (*new) { - shadow->name = (char *)*new; - retval = 0; - goto exit; - } - - _
[PATCH v2 5/7] drivers:hv: Track allocations of children of hv_vmbus in private resource tree
This patch changes vmbus_allocate_mmio() and vmbus_free_mmio() so that when child paravirtual devices allocate memory-mapped I/O space, they allocate it privately from a resource tree pointed at by hyperv_mmio and also by the public resource tree iomem_resource. This allows the region to be marked as "busy" in the private tree, but a "bridge window" in the public tree, guaranteeing that no two bridge windows will overlap each other but while also allowing the PCI device children of the bridge windows to overlap that window. One might conclude that this belongs in the pnp layer, rather than in this driver. Rafael Wysocki, the maintainter of the pnp layer, has previously asked that we not modify the pnp layer as it is considered deprecated. This patch is thus essentially a workaround. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 1ce47d0..dfc6149 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1128,7 +1128,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t size, resource_size_t align, bool fb_overlap_ok) { - struct resource *iter; + struct resource *iter, *shadow; resource_size_t range_min, range_max, start, local_min, local_max; const char *dev_n = dev_name(&device_obj->device); u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); @@ -1170,12 +1170,22 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, start = (local_min + align - 1) & ~(align - 1); for (; start + size - 1 <= local_max; start += align) { + shadow = __request_region(iter, start, + size, + NULL, + IORESOURCE_BUSY); + if (!shadow) + continue; + *new = request_mem_region_exclusive(start, size, dev_n); if (*new) { + shadow->name = (char *)*new; retval = 0; goto exit; } + + __release_region(iter, start, size); } } } @@ -1196,7 +1206,17 @@ EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); */ void vmbus_free_mmio(resource_size_t start, resource_size_t size) { + struct resource *iter; + + down(&hyperv_mmio_lock); + for (iter = hyperv_mmio; iter; iter = iter->sibling) { + if ((iter->start >= start + size) || (iter->end <= start)) + continue; + + __release_region(iter, start, size); + } release_mem_region(start, size); + up(&hyperv_mmio_lock); } EXPORT_SYMBOL_GPL(vmbus_free_mmio); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 0/7] drivers:hv: Ensure that bridge windows don't overlap
This series differs from v2 in that it reserves not only the memory in use as the UEFI frame buffer but also the off-screen memory, so that PCI devices can't reserve that, either. Hyper-V VMs expose paravirtual drivers through a mechanism called VMBus, which is managed by hv_vmbus.ko. For each parvirtual service instance, this driver exposes a new child device. Some of these child devices need memory address space, into which Hyper-V will map things like the virtual video frame buffer. This memory-mapped address space is chosen by the guest OS, not the hypervisor. This is difficult to map onto the Linux pnp layer, as the code in the pnp layer to choose MMIO space keys off of bus type and it doesn't know anything about VMBus. The maintainers of the pnp layer have asked that we not offer patches to it that make it understand VMBus, but that we rather find ways of using the code in its current state. So hv_vmbus.ko exports a function, vmbus_allocate_mmio() for choosing the address space for any child driver that needs this facility. The recently introduced PCI front-end driver for Hyper-V VMs (pci-hyperv.ko) uses vmbus_allocate_mmio() for choosing both the region of memory space into which real PCI Express devices are mapped. The regions allocated are made to look like root PCI bus bridge windows to the PCI driver, reusing all the code in the PCI driver for the rest of PCI device management. The problem is that these bridge windows are marked in such a way that devices can still allocate from the memory space spanned by them, and this means that if two different PCI buses are created in the VM, each with devices under them, they may allocate the same memory space, leading to PCI Base Address Register which overlap. This patch series fixes the problem by tracking allocations to child devices in a separate resource tree, marking them such that the bridge windows can't overlap. The main memory resource tree, iomem_resource, contains resources properly marked as bridge windows, allowing their children to overlap with them. Jake Oshins (7): drivers:hv: Lock access to hyperv_mmio resource tree drivers:hv: Make a function to free mmio regions through vmbus drivers:hv: Use new vmbus_mmio_free() from client drivers. drivers:hv: Reverse order of resources in hyperv_mmio drivers:hv: Track allocations of children of hv_vmbus in private resource tree drivers:hv: Record MMIO range in use by frame buffer drivers:hv: Separate out frame buffer logic when picking MMIO range drivers/hv/vmbus_drv.c | 143 +--- drivers/pci/host/pci-hyperv.c | 14 ++-- drivers/video/fbdev/hyperv_fb.c | 4 +- include/linux/hyperv.h | 2 +- 4 files changed, 115 insertions(+), 48 deletions(-) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 6/7] drivers:hv: Record MMIO range in use by frame buffer
Later in the boot sequence, we need to figure out which memory ranges can be given out to various paravirtual drivers. The hyperv_fb driver should, ideally, be placed right on top of the frame buffer, without some other device getting plopped on top of this range in the meantime. Recording this now allows that to be guaranteed. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 37 - 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index dfc6149..df59bfb 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -41,6 +41,7 @@ #include #include #include +#include #include "hyperv_vmbus.h" static struct acpi_device *hv_acpi_dev; @@ -101,6 +102,8 @@ static struct notifier_block hyperv_panic_block = { .notifier_call = hyperv_panic_event, }; +static const char *fb_mmio_name = "fb_range"; +static struct resource *fb_mmio; struct resource *hyperv_mmio; DEFINE_SEMAPHORE(hyperv_mmio_lock); @@ -1091,6 +1094,12 @@ static int vmbus_acpi_remove(struct acpi_device *device) struct resource *next_res; if (hyperv_mmio) { + if (fb_mmio) { + __release_region(hyperv_mmio, fb_mmio->start, +fb_mmio->end - fb_mmio->start + 1); + fb_mmio = NULL; + } + for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) { next_res = cur_res->sibling; kfree(cur_res); @@ -1100,6 +1109,30 @@ static int vmbus_acpi_remove(struct acpi_device *device) return 0; } +static void vmbus_reserve_fb(void) +{ + int size; + /* +* Make a claim for the frame buffer in the resource tree under the +* first node, which will be the one below 4GB. The length seems to +* be underreported, particularly in a Generation 1 VM. So start out +* reserving a larger area and make it smaller until it succeeds. +*/ + + if (screen_info.lfb_base) { + if (efi_enabled(EFI_BOOT)) + size = max_t(__u32, screen_info.lfb_size, 0x80); + else + size = max_t(__u32, screen_info.lfb_size, 0x400); + + for (; !fb_mmio && (size >= 0x10); size >>= 1) { + fb_mmio = __request_region(hyperv_mmio, + screen_info.lfb_base, size, + fb_mmio_name, 0); + } + } +} + /** * vmbus_allocate_mmio() - Pick a memory-mapped I/O range. * @new: If successful, supplied a pointer to the @@ -1261,8 +1294,10 @@ static int vmbus_acpi_add(struct acpi_device *device) if (ACPI_FAILURE(result)) continue; - if (hyperv_mmio) + if (hyperv_mmio) { + vmbus_reserve_fb(); break; + } } ret_val = 0; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 7/7] drivers:hv: Separate out frame buffer logic when picking MMIO range
Simplify the logic that picks MMIO ranges by pulling out the logic related to trying to lay frame buffer claim on top of where the firmware placed the frame buffer. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 80 ++ 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index df59bfb..bd6d064 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1162,64 +1162,54 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, bool fb_overlap_ok) { struct resource *iter, *shadow; - resource_size_t range_min, range_max, start, local_min, local_max; + resource_size_t range_min, range_max, start; const char *dev_n = dev_name(&device_obj->device); - u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); - int i, retval; + int retval; retval = -ENXIO; down(&hyperv_mmio_lock); + /* +* If overlaps with frame buffers are allowed, then first attempt to +* make the allocation from within the reserved region. Because it +* is already reserved, no shadow allocation is necessary. +*/ + if (fb_overlap_ok && fb_mmio && !(min > fb_mmio->end) && + !(max < fb_mmio->start)) { + + range_min = fb_mmio->start; + range_max = fb_mmio->end; + start = (range_min + align - 1) & ~(align - 1); + for (; start + size - 1 <= range_max; start += align) { + *new = request_mem_region_exclusive(start, size, dev_n); + if (*new) { + retval = 0; + goto exit; + } + } + } + for (iter = hyperv_mmio; iter; iter = iter->sibling) { if ((iter->start >= max) || (iter->end <= min)) continue; range_min = iter->start; range_max = iter->end; - - /* If this range overlaps the frame buffer, split it into - two tries. */ - for (i = 0; i < 2; i++) { - local_min = range_min; - local_max = range_max; - if (fb_overlap_ok || (range_min >= fb_end) || - (range_max <= screen_info.lfb_base)) { - i++; - } else { - if ((range_min <= screen_info.lfb_base) && - (range_max >= screen_info.lfb_base)) { - /* -* The frame buffer is in this window, -* so trim this into the part that -* preceeds the frame buffer. -*/ - local_max = screen_info.lfb_base - 1; - range_min = fb_end; - } else { - range_min = fb_end; - continue; - } + start = (range_min + align - 1) & ~(align - 1); + for (; start + size - 1 <= range_max; start += align) { + shadow = __request_region(iter, start, size, NULL, + IORESOURCE_BUSY); + if (!shadow) + continue; + + *new = request_mem_region_exclusive(start, size, dev_n); + if (*new) { + shadow->name = (char *)*new; + retval = 0; + goto exit; } - start = (local_min + align - 1) & ~(align - 1); - for (; start + size - 1 <= local_max; start += align) { - shadow = __request_region(iter, start, - size, - NULL, - IORESOURCE_BUSY); - if (!shadow) - continue; - - *new = request_mem_region_exclusive(start, size, - dev_n); - if (*new) { - shadow->name = (char *)*new; - retval = 0; -
[PATCH v3 5/7] drivers:hv: Track allocations of children of hv_vmbus in private resource tree
This patch changes vmbus_allocate_mmio() and vmbus_free_mmio() so that when child paravirtual devices allocate memory-mapped I/O space, they allocate it privately from a resource tree pointed at by hyperv_mmio and also by the public resource tree iomem_resource. This allows the region to be marked as "busy" in the private tree, but a "bridge window" in the public tree, guaranteeing that no two bridge windows will overlap each other but while also allowing the PCI device children of the bridge windows to overlap that window. One might conclude that this belongs in the pnp layer, rather than in this driver. Rafael Wysocki, the maintainter of the pnp layer, has previously asked that we not modify the pnp layer as it is considered deprecated. This patch is thus essentially a workaround. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 1ce47d0..dfc6149 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1128,7 +1128,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t size, resource_size_t align, bool fb_overlap_ok) { - struct resource *iter; + struct resource *iter, *shadow; resource_size_t range_min, range_max, start, local_min, local_max; const char *dev_n = dev_name(&device_obj->device); u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); @@ -1170,12 +1170,22 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, start = (local_min + align - 1) & ~(align - 1); for (; start + size - 1 <= local_max; start += align) { + shadow = __request_region(iter, start, + size, + NULL, + IORESOURCE_BUSY); + if (!shadow) + continue; + *new = request_mem_region_exclusive(start, size, dev_n); if (*new) { + shadow->name = (char *)*new; retval = 0; goto exit; } + + __release_region(iter, start, size); } } } @@ -1196,7 +1206,17 @@ EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); */ void vmbus_free_mmio(resource_size_t start, resource_size_t size) { + struct resource *iter; + + down(&hyperv_mmio_lock); + for (iter = hyperv_mmio; iter; iter = iter->sibling) { + if ((iter->start >= start + size) || (iter->end <= start)) + continue; + + __release_region(iter, start, size); + } release_mem_region(start, size); + up(&hyperv_mmio_lock); } EXPORT_SYMBOL_GPL(vmbus_free_mmio); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/7] drivers:hv: Make a function to free mmio regions through vmbus
This patch introduces a function that reverses everything done by vmbus_allocate_mmio(). Existing code just called release_mem_region(). Future patches in this series require a more complex sequence of actions, so this function is introduced to wrap those actions. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 15 +++ include/linux/hyperv.h | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 799518b..60553c1 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1188,6 +1188,21 @@ exit: EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); /** + * vmbus_free_mmio() - Free a memory-mapped I/O range. + * @start: Base address of region to release. + * @size: Size of the range to be allocated + * + * This function releases anything requested by + * vmbus_mmio_allocate(). + */ +void vmbus_free_mmio(resource_size_t start, resource_size_t size) +{ + release_mem_region(start, size); + +} +EXPORT_SYMBOL_GPL(vmbus_free_mmio); + +/** * vmbus_cpu_number_to_vp_number() - Map CPU to VP. * @cpu_number: CPU number in Linux terms * diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index aa0fadc..ecd81c3 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1091,7 +1091,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t min, resource_size_t max, resource_size_t size, resource_size_t align, bool fb_overlap_ok); - +void vmbus_free_mmio(resource_size_t start, resource_size_t size); int vmbus_cpu_number_to_vp_number(int cpu_number); u64 hv_do_hypercall(u64 control, void *input, void *output); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/7] drivers:hv: Lock access to hyperv_mmio resource tree
In existing code, this tree of resources is created in single-threaded code and never modified after it is created, and thus needs no locking. This patch introduces a semaphore for tree access, as other patches in this series introduce run-time modifications of this resource tree which can happen on multiple threads. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 64713ff..799518b 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -102,6 +102,7 @@ static struct notifier_block hyperv_panic_block = { }; struct resource *hyperv_mmio; +DEFINE_SEMAPHORE(hyperv_mmio_lock); static int vmbus_exists(void) { @@ -1132,7 +1133,10 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t range_min, range_max, start, local_min, local_max; const char *dev_n = dev_name(&device_obj->device); u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); - int i; + int i, retval; + + retval = -ENXIO; + down(&hyperv_mmio_lock); for (iter = hyperv_mmio; iter; iter = iter->sibling) { if ((iter->start >= max) || (iter->end <= min)) @@ -1169,13 +1173,17 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, for (; start + size - 1 <= local_max; start += align) { *new = request_mem_region_exclusive(start, size, dev_n); - if (*new) - return 0; + if (*new) { + retval = 0; + goto exit; + } } } } - return -ENXIO; +exit: + up(&hyperv_mmio_lock); + return retval; } EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 4/7] drivers:hv: Reverse order of resources in hyperv_mmio
A patch later in this series allocates child nodes in this resource tree. For that to work, this tree needs to be sorted in ascending order. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 60553c1..1ce47d0 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1049,7 +1049,6 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) new_res->end = end; /* -* Stick ranges from higher in address space at the front of the list. * If two ranges are adjacent, merge them. */ do { @@ -1070,7 +1069,7 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) break; } - if ((*old_res)->end < new_res->start) { + if ((*old_res)->start > new_res->end) { new_res->sibling = *old_res; if (prev_res) (*prev_res)->sibling = new_res; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.
This patch modifies all the callers of vmbus_mmio_allocate() to call vmbus_mmio_free() instead of release_mem_region(). Signed-off-by: Jake Oshins --- drivers/pci/host/pci-hyperv.c | 14 +++--- drivers/video/fbdev/hyperv_fb.c | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index ed651ba..f2559b6 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -1795,14 +1795,14 @@ static void hv_pci_free_bridge_windows(struct hv_pcibus_device *hbus) if (hbus->low_mmio_space && hbus->low_mmio_res) { hbus->low_mmio_res->flags |= IORESOURCE_BUSY; - release_mem_region(hbus->low_mmio_res->start, - resource_size(hbus->low_mmio_res)); + vmbus_free_mmio(hbus->low_mmio_res->start, + resource_size(hbus->low_mmio_res)); } if (hbus->high_mmio_space && hbus->high_mmio_res) { hbus->high_mmio_res->flags |= IORESOURCE_BUSY; - release_mem_region(hbus->high_mmio_res->start, - resource_size(hbus->high_mmio_res)); + vmbus_free_mmio(hbus->high_mmio_res->start, + resource_size(hbus->high_mmio_res)); } } @@ -1880,8 +1880,8 @@ static int hv_pci_allocate_bridge_windows(struct hv_pcibus_device *hbus) release_low_mmio: if (hbus->low_mmio_res) { - release_mem_region(hbus->low_mmio_res->start, - resource_size(hbus->low_mmio_res)); + vmbus_free_mmio(hbus->low_mmio_res->start, + resource_size(hbus->low_mmio_res)); } return ret; @@ -1924,7 +1924,7 @@ static int hv_allocate_config_window(struct hv_pcibus_device *hbus) static void hv_free_config_window(struct hv_pcibus_device *hbus) { - release_mem_region(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH); + vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH); } /** diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index e2451bd..2fd49b2 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -743,7 +743,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) err3: iounmap(fb_virt); err2: - release_mem_region(par->mem->start, screen_fb_size); + vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL; err1: if (!gen2vm) @@ -758,7 +758,7 @@ static void hvfb_putmem(struct fb_info *info) struct hvfb_par *par = info->par; iounmap(info->screen_base); - release_mem_region(par->mem->start, screen_fb_size); + vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL; } -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.
This patch modifies all the callers of vmbus_mmio_allocate() to call vmbus_mmio_free() instead of release_mem_region(). Signed-off-by: Jake Oshins --- drivers/pci/host/pci-hyperv.c | 14 +++--- drivers/video/fbdev/hyperv_fb.c | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index ed651ba..f2559b6 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -1795,14 +1795,14 @@ static void hv_pci_free_bridge_windows(struct hv_pcibus_device *hbus) if (hbus->low_mmio_space && hbus->low_mmio_res) { hbus->low_mmio_res->flags |= IORESOURCE_BUSY; - release_mem_region(hbus->low_mmio_res->start, - resource_size(hbus->low_mmio_res)); + vmbus_free_mmio(hbus->low_mmio_res->start, + resource_size(hbus->low_mmio_res)); } if (hbus->high_mmio_space && hbus->high_mmio_res) { hbus->high_mmio_res->flags |= IORESOURCE_BUSY; - release_mem_region(hbus->high_mmio_res->start, - resource_size(hbus->high_mmio_res)); + vmbus_free_mmio(hbus->high_mmio_res->start, + resource_size(hbus->high_mmio_res)); } } @@ -1880,8 +1880,8 @@ static int hv_pci_allocate_bridge_windows(struct hv_pcibus_device *hbus) release_low_mmio: if (hbus->low_mmio_res) { - release_mem_region(hbus->low_mmio_res->start, - resource_size(hbus->low_mmio_res)); + vmbus_free_mmio(hbus->low_mmio_res->start, + resource_size(hbus->low_mmio_res)); } return ret; @@ -1924,7 +1924,7 @@ static int hv_allocate_config_window(struct hv_pcibus_device *hbus) static void hv_free_config_window(struct hv_pcibus_device *hbus) { - release_mem_region(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH); + vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH); } /** diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index e2451bd..2fd49b2 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -743,7 +743,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) err3: iounmap(fb_virt); err2: - release_mem_region(par->mem->start, screen_fb_size); + vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL; err1: if (!gen2vm) @@ -758,7 +758,7 @@ static void hvfb_putmem(struct fb_info *info) struct hvfb_par *par = info->par; iounmap(info->screen_base); - release_mem_region(par->mem->start, screen_fb_size); + vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL; } -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 7/7] drivers:hv: Separate out frame buffer logic when picking MMIO range
Simplify the logic that picks MMIO ranges by pulling out the logic related to trying to lay frame buffer claim on top of where the firmware placed the frame buffer. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 80 ++ 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index eaa5c3b..a29a6c0 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1162,64 +1162,54 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, bool fb_overlap_ok) { struct resource *iter, *shadow; - resource_size_t range_min, range_max, start, local_min, local_max; + resource_size_t range_min, range_max, start; const char *dev_n = dev_name(&device_obj->device); - u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); - int i, retval; + int retval; retval = -ENXIO; down(&hyperv_mmio_lock); + /* +* If overlaps with frame buffers are allowed, then first attempt to +* make the allocation from within the reserved region. Because it +* is already reserved, no shadow allocation is necessary. +*/ + if (fb_overlap_ok && fb_mmio && !(min > fb_mmio->end) && + !(max < fb_mmio->start)) { + + range_min = fb_mmio->start; + range_max = fb_mmio->end; + start = (range_min + align - 1) & ~(align - 1); + for (; start + size - 1 <= range_max; start += align) { + *new = request_mem_region_exclusive(start, size, dev_n); + if (*new) { + retval = 0; + goto exit; + } + } + } + for (iter = hyperv_mmio; iter; iter = iter->sibling) { if ((iter->start >= max) || (iter->end <= min)) continue; range_min = iter->start; range_max = iter->end; - - /* If this range overlaps the frame buffer, split it into - two tries. */ - for (i = 0; i < 2; i++) { - local_min = range_min; - local_max = range_max; - if (fb_overlap_ok || (range_min >= fb_end) || - (range_max <= screen_info.lfb_base)) { - i++; - } else { - if ((range_min <= screen_info.lfb_base) && - (range_max >= screen_info.lfb_base)) { - /* -* The frame buffer is in this window, -* so trim this into the part that -* preceeds the frame buffer. -*/ - local_max = screen_info.lfb_base - 1; - range_min = fb_end; - } else { - range_min = fb_end; - continue; - } + start = (range_min + align - 1) & ~(align - 1); + for (; start + size - 1 <= range_max; start += align) { + shadow = __request_region(iter, start, size, NULL, + IORESOURCE_BUSY); + if (!shadow) + continue; + + *new = request_mem_region_exclusive(start, size, dev_n); + if (*new) { + shadow->name = (char *)*new; + retval = 0; + goto exit; } - start = (local_min + align - 1) & ~(align - 1); - for (; start + size - 1 <= local_max; start += align) { - shadow = __request_region(iter, start, - size, - NULL, - IORESOURCE_BUSY); - if (!shadow) - continue; - - *new = request_mem_region_exclusive(start, size, - dev_n); - if (*new) { - shadow->name = (char *)*new; - retval = 0; -
[PATCH v4 4/7] drivers:hv: Reverse order of resources in hyperv_mmio
A patch later in this series allocates child nodes in this resource tree. For that to work, this tree needs to be sorted in ascending order. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 60553c1..1ce47d0 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1049,7 +1049,6 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) new_res->end = end; /* -* Stick ranges from higher in address space at the front of the list. * If two ranges are adjacent, merge them. */ do { @@ -1070,7 +1069,7 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) break; } - if ((*old_res)->end < new_res->start) { + if ((*old_res)->start > new_res->end) { new_res->sibling = *old_res; if (prev_res) (*prev_res)->sibling = new_res; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 2/7] drivers:hv: Make a function to free mmio regions through vmbus
This patch introduces a function that reverses everything done by vmbus_allocate_mmio(). Existing code just called release_mem_region(). Future patches in this series require a more complex sequence of actions, so this function is introduced to wrap those actions. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 15 +++ include/linux/hyperv.h | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 799518b..60553c1 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1188,6 +1188,21 @@ exit: EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); /** + * vmbus_free_mmio() - Free a memory-mapped I/O range. + * @start: Base address of region to release. + * @size: Size of the range to be allocated + * + * This function releases anything requested by + * vmbus_mmio_allocate(). + */ +void vmbus_free_mmio(resource_size_t start, resource_size_t size) +{ + release_mem_region(start, size); + +} +EXPORT_SYMBOL_GPL(vmbus_free_mmio); + +/** * vmbus_cpu_number_to_vp_number() - Map CPU to VP. * @cpu_number: CPU number in Linux terms * diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index aa0fadc..ecd81c3 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1091,7 +1091,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t min, resource_size_t max, resource_size_t size, resource_size_t align, bool fb_overlap_ok); - +void vmbus_free_mmio(resource_size_t start, resource_size_t size); int vmbus_cpu_number_to_vp_number(int cpu_number); u64 hv_do_hypercall(u64 control, void *input, void *output); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 6/7] drivers:hv: Record MMIO range in use by frame buffer
Later in the boot sequence, we need to figure out which memory ranges can be given out to various paravirtual drivers. The hyperv_fb driver should, ideally, be placed right on top of the frame buffer, without some other device getting plopped on top of this range in the meantime. Recording this now allows that to be guaranteed. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 37 - 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index dfc6149..eaa5c3b 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -41,6 +41,7 @@ #include #include #include +#include #include "hyperv_vmbus.h" static struct acpi_device *hv_acpi_dev; @@ -101,6 +102,8 @@ static struct notifier_block hyperv_panic_block = { .notifier_call = hyperv_panic_event, }; +static const char *fb_mmio_name = "fb_range"; +static struct resource *fb_mmio; struct resource *hyperv_mmio; DEFINE_SEMAPHORE(hyperv_mmio_lock); @@ -1091,6 +1094,12 @@ static int vmbus_acpi_remove(struct acpi_device *device) struct resource *next_res; if (hyperv_mmio) { + if (fb_mmio) { + __release_region(hyperv_mmio, fb_mmio->start, +resource_size(fb_mmio)); + fb_mmio = NULL; + } + for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) { next_res = cur_res->sibling; kfree(cur_res); @@ -1100,6 +1109,30 @@ static int vmbus_acpi_remove(struct acpi_device *device) return 0; } +static void vmbus_reserve_fb(void) +{ + int size; + /* +* Make a claim for the frame buffer in the resource tree under the +* first node, which will be the one below 4GB. The length seems to +* be underreported, particularly in a Generation 1 VM. So start out +* reserving a larger area and make it smaller until it succeeds. +*/ + + if (screen_info.lfb_base) { + if (efi_enabled(EFI_BOOT)) + size = max_t(__u32, screen_info.lfb_size, 0x80); + else + size = max_t(__u32, screen_info.lfb_size, 0x400); + + for (; !fb_mmio && (size >= 0x10); size >>= 1) { + fb_mmio = __request_region(hyperv_mmio, + screen_info.lfb_base, size, + fb_mmio_name, 0); + } + } +} + /** * vmbus_allocate_mmio() - Pick a memory-mapped I/O range. * @new: If successful, supplied a pointer to the @@ -1261,8 +1294,10 @@ static int vmbus_acpi_add(struct acpi_device *device) if (ACPI_FAILURE(result)) continue; - if (hyperv_mmio) + if (hyperv_mmio) { + vmbus_reserve_fb(); break; + } } ret_val = 0; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 5/7] drivers:hv: Track allocations of children of hv_vmbus in private resource tree
This patch changes vmbus_allocate_mmio() and vmbus_free_mmio() so that when child paravirtual devices allocate memory-mapped I/O space, they allocate it privately from a resource tree pointed at by hyperv_mmio and also by the public resource tree iomem_resource. This allows the region to be marked as "busy" in the private tree, but a "bridge window" in the public tree, guaranteeing that no two bridge windows will overlap each other but while also allowing the PCI device children of the bridge windows to overlap that window. One might conclude that this belongs in the pnp layer, rather than in this driver. Rafael Wysocki, the maintainter of the pnp layer, has previously asked that we not modify the pnp layer as it is considered deprecated. This patch is thus essentially a workaround. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 1ce47d0..dfc6149 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1128,7 +1128,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t size, resource_size_t align, bool fb_overlap_ok) { - struct resource *iter; + struct resource *iter, *shadow; resource_size_t range_min, range_max, start, local_min, local_max; const char *dev_n = dev_name(&device_obj->device); u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); @@ -1170,12 +1170,22 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, start = (local_min + align - 1) & ~(align - 1); for (; start + size - 1 <= local_max; start += align) { + shadow = __request_region(iter, start, + size, + NULL, + IORESOURCE_BUSY); + if (!shadow) + continue; + *new = request_mem_region_exclusive(start, size, dev_n); if (*new) { + shadow->name = (char *)*new; retval = 0; goto exit; } + + __release_region(iter, start, size); } } } @@ -1196,7 +1206,17 @@ EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); */ void vmbus_free_mmio(resource_size_t start, resource_size_t size) { + struct resource *iter; + + down(&hyperv_mmio_lock); + for (iter = hyperv_mmio; iter; iter = iter->sibling) { + if ((iter->start >= start + size) || (iter->end <= start)) + continue; + + __release_region(iter, start, size); + } release_mem_region(start, size); + up(&hyperv_mmio_lock); } EXPORT_SYMBOL_GPL(vmbus_free_mmio); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 1/7] drivers:hv: Lock access to hyperv_mmio resource tree
In existing code, this tree of resources is created in single-threaded code and never modified after it is created, and thus needs no locking. This patch introduces a semaphore for tree access, as other patches in this series introduce run-time modifications of this resource tree which can happen on multiple threads. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 64713ff..799518b 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -102,6 +102,7 @@ static struct notifier_block hyperv_panic_block = { }; struct resource *hyperv_mmio; +DEFINE_SEMAPHORE(hyperv_mmio_lock); static int vmbus_exists(void) { @@ -1132,7 +1133,10 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t range_min, range_max, start, local_min, local_max; const char *dev_n = dev_name(&device_obj->device); u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); - int i; + int i, retval; + + retval = -ENXIO; + down(&hyperv_mmio_lock); for (iter = hyperv_mmio; iter; iter = iter->sibling) { if ((iter->start >= max) || (iter->end <= min)) @@ -1169,13 +1173,17 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, for (; start + size - 1 <= local_max; start += align) { *new = request_mem_region_exclusive(start, size, dev_n); - if (*new) - return 0; + if (*new) { + retval = 0; + goto exit; + } } } } - return -ENXIO; +exit: + up(&hyperv_mmio_lock); + return retval; } EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 0/7] drivers:hv: Ensure that bridge windows don't overlap
This series differs from v3 in that it folds in a patch suggested by the kbuild test robot, substituting resource_size() for directly calculating the size. Hyper-V VMs expose paravirtual drivers through a mechanism called VMBus, which is managed by hv_vmbus.ko. For each parvirtual service instance, this driver exposes a new child device. Some of these child devices need memory address space, into which Hyper-V will map things like the virtual video frame buffer. This memory-mapped address space is chosen by the guest OS, not the hypervisor. This is difficult to map onto the Linux pnp layer, as the code in the pnp layer to choose MMIO space keys off of bus type and it doesn't know anything about VMBus. The maintainers of the pnp layer have asked that we not offer patches to it that make it understand VMBus, but that we rather find ways of using the code in its current state. So hv_vmbus.ko exports a function, vmbus_allocate_mmio() for choosing the address space for any child driver that needs this facility. The recently introduced PCI front-end driver for Hyper-V VMs (pci-hyperv.ko) uses vmbus_allocate_mmio() for choosing both the region of memory space into which real PCI Express devices are mapped. The regions allocated are made to look like root PCI bus bridge windows to the PCI driver, reusing all the code in the PCI driver for the rest of PCI device management. The problem is that these bridge windows are marked in such a way that devices can still allocate from the memory space spanned by them, and this means that if two different PCI buses are created in the VM, each with devices under them, they may allocate the same memory space, leading to PCI Base Address Register which overlap. This patch series fixes the problem by tracking allocations to child devices in a separate resource tree, marking them such that the bridge windows can't overlap. The main memory resource tree, iomem_resource, contains resources properly marked as bridge windows, allowing their children to overlap with them. Jake Oshins (7): drivers:hv: Lock access to hyperv_mmio resource tree drivers:hv: Make a function to free mmio regions through vmbus drivers:hv: Use new vmbus_mmio_free() from client drivers. drivers:hv: Reverse order of resources in hyperv_mmio drivers:hv: Track allocations of children of hv_vmbus in private resource tree drivers:hv: Record MMIO range in use by frame buffer drivers:hv: Separate out frame buffer logic when picking MMIO range drivers/hv/vmbus_drv.c | 143 +--- drivers/pci/host/pci-hyperv.c | 14 ++-- drivers/video/fbdev/hyperv_fb.c | 4 +- include/linux/hyperv.h | 2 +- 4 files changed, 115 insertions(+), 48 deletions(-) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v4 3/7] drivers:hv: Use new vmbus_mmio_free() from client drivers.
> -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: Tuesday, April 5, 2016 11:00 AM > To: Jake Oshins > Cc: linux-...@vger.kernel.org; gre...@linuxfoundation.org; KY Srinivasan > ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; Haiyang Zhang ; Hadden > Hoppert > Subject: Re: [PATCH v4 3/7] drivers:hv: Use new vmbus_mmio_free() from > client drivers. > > Hi Jake, > > On Fri, Apr 01, 2016 at 05:47:43PM -0700, Jake Oshins wrote: > > This patch modifies all the callers of vmbus_mmio_allocate() > > to call vmbus_mmio_free() instead of release_mem_region(). > > This changelog merely restates the C code. Presumably there's some > important difference between release_mem_region() and > vmbus_mmio_free(), and we need a hint about what that is. > > Oh, I see, there actually is no difference *yet*, but it's coming. > I'd combine this with patch 2. Then the patch is obviously correct > all by itself, and the changelog for patch 2 makes clear what's > happening. > > In changelogs, don't bother with "this patch does" or "this function > is introduced." The context is obvious because the changelog is part > of the commit. Write imperative sentences, e.g., "Call > vmbus_mmio_free() instead of release_mem_region()." > Will do. Thanks. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 1/6] drivers:hv: Lock access to hyperv_mmio resource tree
In existing code, this tree of resources is created in single-threaded code and never modified after it is created, and thus needs no locking. This patch introduces a semaphore for tree access, as other patches in this series introduce run-time modifications of this resource tree which can happen on multiple threads. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 64713ff..799518b 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -102,6 +102,7 @@ static struct notifier_block hyperv_panic_block = { }; struct resource *hyperv_mmio; +DEFINE_SEMAPHORE(hyperv_mmio_lock); static int vmbus_exists(void) { @@ -1132,7 +1133,10 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t range_min, range_max, start, local_min, local_max; const char *dev_n = dev_name(&device_obj->device); u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); - int i; + int i, retval; + + retval = -ENXIO; + down(&hyperv_mmio_lock); for (iter = hyperv_mmio; iter; iter = iter->sibling) { if ((iter->start >= max) || (iter->end <= min)) @@ -1169,13 +1173,17 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, for (; start + size - 1 <= local_max; start += align) { *new = request_mem_region_exclusive(start, size, dev_n); - if (*new) - return 0; + if (*new) { + retval = 0; + goto exit; + } } } } - return -ENXIO; +exit: + up(&hyperv_mmio_lock); + return retval; } EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 3/6] drivers:hv: Reverse order of resources in hyperv_mmio
A patch later in this series allocates child nodes in this resource tree. For that to work, this tree needs to be sorted in ascending order. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 60553c1..1ce47d0 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1049,7 +1049,6 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) new_res->end = end; /* -* Stick ranges from higher in address space at the front of the list. * If two ranges are adjacent, merge them. */ do { @@ -1070,7 +1069,7 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) break; } - if ((*old_res)->end < new_res->start) { + if ((*old_res)->start > new_res->end) { new_res->sibling = *old_res; if (prev_res) (*prev_res)->sibling = new_res; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 0/6] drivers:hv: Ensure that bridge windows don't overlap
This version incorporates feedback from Bjorn Helgaas, folding two patches together. Hyper-V VMs expose paravirtual drivers through a mechanism called VMBus, which is managed by hv_vmbus.ko. For each parvirtual service instance, this driver exposes a new child device. Some of these child devices need memory address space, into which Hyper-V will map things like the virtual video frame buffer. This memory-mapped address space is chosen by the guest OS, not the hypervisor. This is difficult to map onto the Linux pnp layer, as the code in the pnp layer to choose MMIO space keys off of bus type and it doesn't know anything about VMBus. The maintainers of the pnp layer have asked that we not offer patches to it that make it understand VMBus, but that we rather find ways of using the code in its current state. So hv_vmbus.ko exports a function, vmbus_allocate_mmio() for choosing the address space for any child driver that needs this facility. The recently introduced PCI front-end driver for Hyper-V VMs (pci-hyperv.ko) uses vmbus_allocate_mmio() for choosing both the region of memory space into which real PCI Express devices are mapped. The regions allocated are made to look like root PCI bus bridge windows to the PCI driver, reusing all the code in the PCI driver for the rest of PCI device management. The problem is that these bridge windows are marked in such a way that devices can still allocate from the memory space spanned by them, and this means that if two different PCI buses are created in the VM, each with devices under them, they may allocate the same memory space, leading to PCI Base Address Register which overlap. This patch series fixes the problem by tracking allocations to child devices in a separate resource tree, marking them such that the bridge windows can't overlap. The main memory resource tree, iomem_resource, contains resources properly marked as bridge windows, allowing their children to overlap with them. Jake Oshins (6): drivers:hv: Lock access to hyperv_mmio resource tree drivers:hv: Call vmbus_mmio_free() to reverse vmbus_mmio_allocate() drivers:hv: Reverse order of resources in hyperv_mmio drivers:hv: Track allocations of children of hv_vmbus in private resource tree drivers:hv: Record MMIO range in use by frame buffer drivers:hv: Separate out frame buffer logic when picking MMIO range drivers/hv/vmbus_drv.c | 143 +--- drivers/pci/host/pci-hyperv.c | 14 ++-- drivers/video/fbdev/hyperv_fb.c | 4 +- include/linux/hyperv.h | 2 +- 4 files changed, 115 insertions(+), 48 deletions(-) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 4/6] drivers:hv: Track allocations of children of hv_vmbus in private resource tree
This patch changes vmbus_allocate_mmio() and vmbus_free_mmio() so that when child paravirtual devices allocate memory-mapped I/O space, they allocate it privately from a resource tree pointed at by hyperv_mmio and also by the public resource tree iomem_resource. This allows the region to be marked as "busy" in the private tree, but a "bridge window" in the public tree, guaranteeing that no two bridge windows will overlap each other but while also allowing the PCI device children of the bridge windows to overlap that window. One might conclude that this belongs in the pnp layer, rather than in this driver. Rafael Wysocki, the maintainter of the pnp layer, has previously asked that we not modify the pnp layer as it is considered deprecated. This patch is thus essentially a workaround. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 22 +- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 1ce47d0..dfc6149 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1128,7 +1128,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t size, resource_size_t align, bool fb_overlap_ok) { - struct resource *iter; + struct resource *iter, *shadow; resource_size_t range_min, range_max, start, local_min, local_max; const char *dev_n = dev_name(&device_obj->device); u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); @@ -1170,12 +1170,22 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, start = (local_min + align - 1) & ~(align - 1); for (; start + size - 1 <= local_max; start += align) { + shadow = __request_region(iter, start, + size, + NULL, + IORESOURCE_BUSY); + if (!shadow) + continue; + *new = request_mem_region_exclusive(start, size, dev_n); if (*new) { + shadow->name = (char *)*new; retval = 0; goto exit; } + + __release_region(iter, start, size); } } } @@ -1196,7 +1206,17 @@ EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); */ void vmbus_free_mmio(resource_size_t start, resource_size_t size) { + struct resource *iter; + + down(&hyperv_mmio_lock); + for (iter = hyperv_mmio; iter; iter = iter->sibling) { + if ((iter->start >= start + size) || (iter->end <= start)) + continue; + + __release_region(iter, start, size); + } release_mem_region(start, size); + up(&hyperv_mmio_lock); } EXPORT_SYMBOL_GPL(vmbus_free_mmio); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 6/6] drivers:hv: Separate out frame buffer logic when picking MMIO range
Simplify the logic that picks MMIO ranges by pulling out the logic related to trying to lay frame buffer claim on top of where the firmware placed the frame buffer. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 80 ++ 1 file changed, 35 insertions(+), 45 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index eaa5c3b..a29a6c0 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1162,64 +1162,54 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, bool fb_overlap_ok) { struct resource *iter, *shadow; - resource_size_t range_min, range_max, start, local_min, local_max; + resource_size_t range_min, range_max, start; const char *dev_n = dev_name(&device_obj->device); - u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); - int i, retval; + int retval; retval = -ENXIO; down(&hyperv_mmio_lock); + /* +* If overlaps with frame buffers are allowed, then first attempt to +* make the allocation from within the reserved region. Because it +* is already reserved, no shadow allocation is necessary. +*/ + if (fb_overlap_ok && fb_mmio && !(min > fb_mmio->end) && + !(max < fb_mmio->start)) { + + range_min = fb_mmio->start; + range_max = fb_mmio->end; + start = (range_min + align - 1) & ~(align - 1); + for (; start + size - 1 <= range_max; start += align) { + *new = request_mem_region_exclusive(start, size, dev_n); + if (*new) { + retval = 0; + goto exit; + } + } + } + for (iter = hyperv_mmio; iter; iter = iter->sibling) { if ((iter->start >= max) || (iter->end <= min)) continue; range_min = iter->start; range_max = iter->end; - - /* If this range overlaps the frame buffer, split it into - two tries. */ - for (i = 0; i < 2; i++) { - local_min = range_min; - local_max = range_max; - if (fb_overlap_ok || (range_min >= fb_end) || - (range_max <= screen_info.lfb_base)) { - i++; - } else { - if ((range_min <= screen_info.lfb_base) && - (range_max >= screen_info.lfb_base)) { - /* -* The frame buffer is in this window, -* so trim this into the part that -* preceeds the frame buffer. -*/ - local_max = screen_info.lfb_base - 1; - range_min = fb_end; - } else { - range_min = fb_end; - continue; - } + start = (range_min + align - 1) & ~(align - 1); + for (; start + size - 1 <= range_max; start += align) { + shadow = __request_region(iter, start, size, NULL, + IORESOURCE_BUSY); + if (!shadow) + continue; + + *new = request_mem_region_exclusive(start, size, dev_n); + if (*new) { + shadow->name = (char *)*new; + retval = 0; + goto exit; } - start = (local_min + align - 1) & ~(align - 1); - for (; start + size - 1 <= local_max; start += align) { - shadow = __request_region(iter, start, - size, - NULL, - IORESOURCE_BUSY); - if (!shadow) - continue; - - *new = request_mem_region_exclusive(start, size, - dev_n); - if (*new) { - shadow->name = (char *)*new; - retval = 0; -
[PATCH v5 2/6] drivers:hv: Call vmbus_mmio_free() to reverse vmbus_mmio_allocate()
Existing code just called release_mem_region(). Adding a wrapper around it allows the more complex range tracking that is introduced later in this patch series. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 15 +++ drivers/pci/host/pci-hyperv.c | 14 +++--- drivers/video/fbdev/hyperv_fb.c | 4 ++-- include/linux/hyperv.h | 2 +- 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 799518b..60553c1 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1188,6 +1188,21 @@ exit: EXPORT_SYMBOL_GPL(vmbus_allocate_mmio); /** + * vmbus_free_mmio() - Free a memory-mapped I/O range. + * @start: Base address of region to release. + * @size: Size of the range to be allocated + * + * This function releases anything requested by + * vmbus_mmio_allocate(). + */ +void vmbus_free_mmio(resource_size_t start, resource_size_t size) +{ + release_mem_region(start, size); + +} +EXPORT_SYMBOL_GPL(vmbus_free_mmio); + +/** * vmbus_cpu_number_to_vp_number() - Map CPU to VP. * @cpu_number: CPU number in Linux terms * diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c index ed651ba..f2559b6 100644 --- a/drivers/pci/host/pci-hyperv.c +++ b/drivers/pci/host/pci-hyperv.c @@ -1795,14 +1795,14 @@ static void hv_pci_free_bridge_windows(struct hv_pcibus_device *hbus) if (hbus->low_mmio_space && hbus->low_mmio_res) { hbus->low_mmio_res->flags |= IORESOURCE_BUSY; - release_mem_region(hbus->low_mmio_res->start, - resource_size(hbus->low_mmio_res)); + vmbus_free_mmio(hbus->low_mmio_res->start, + resource_size(hbus->low_mmio_res)); } if (hbus->high_mmio_space && hbus->high_mmio_res) { hbus->high_mmio_res->flags |= IORESOURCE_BUSY; - release_mem_region(hbus->high_mmio_res->start, - resource_size(hbus->high_mmio_res)); + vmbus_free_mmio(hbus->high_mmio_res->start, + resource_size(hbus->high_mmio_res)); } } @@ -1880,8 +1880,8 @@ static int hv_pci_allocate_bridge_windows(struct hv_pcibus_device *hbus) release_low_mmio: if (hbus->low_mmio_res) { - release_mem_region(hbus->low_mmio_res->start, - resource_size(hbus->low_mmio_res)); + vmbus_free_mmio(hbus->low_mmio_res->start, + resource_size(hbus->low_mmio_res)); } return ret; @@ -1924,7 +1924,7 @@ static int hv_allocate_config_window(struct hv_pcibus_device *hbus) static void hv_free_config_window(struct hv_pcibus_device *hbus) { - release_mem_region(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH); + vmbus_free_mmio(hbus->mem_config->start, PCI_CONFIG_MMIO_LENGTH); } /** diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index e2451bd..2fd49b2 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -743,7 +743,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) err3: iounmap(fb_virt); err2: - release_mem_region(par->mem->start, screen_fb_size); + vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL; err1: if (!gen2vm) @@ -758,7 +758,7 @@ static void hvfb_putmem(struct fb_info *info) struct hvfb_par *par = info->par; iounmap(info->screen_base); - release_mem_region(par->mem->start, screen_fb_size); + vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL; } diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index aa0fadc..ecd81c3 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1091,7 +1091,7 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj, resource_size_t min, resource_size_t max, resource_size_t size, resource_size_t align, bool fb_overlap_ok); - +void vmbus_free_mmio(resource_size_t start, resource_size_t size); int vmbus_cpu_number_to_vp_number(int cpu_number); u64 hv_do_hypercall(u64 control, void *input, void *output); -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v5 5/6] drivers:hv: Record MMIO range in use by frame buffer
Later in the boot sequence, we need to figure out which memory ranges can be given out to various paravirtual drivers. The hyperv_fb driver should, ideally, be placed right on top of the frame buffer, without some other device getting plopped on top of this range in the meantime. Recording this now allows that to be guaranteed. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 37 - 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index dfc6149..eaa5c3b 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -41,6 +41,7 @@ #include #include #include +#include #include "hyperv_vmbus.h" static struct acpi_device *hv_acpi_dev; @@ -101,6 +102,8 @@ static struct notifier_block hyperv_panic_block = { .notifier_call = hyperv_panic_event, }; +static const char *fb_mmio_name = "fb_range"; +static struct resource *fb_mmio; struct resource *hyperv_mmio; DEFINE_SEMAPHORE(hyperv_mmio_lock); @@ -1091,6 +1094,12 @@ static int vmbus_acpi_remove(struct acpi_device *device) struct resource *next_res; if (hyperv_mmio) { + if (fb_mmio) { + __release_region(hyperv_mmio, fb_mmio->start, +resource_size(fb_mmio)); + fb_mmio = NULL; + } + for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) { next_res = cur_res->sibling; kfree(cur_res); @@ -1100,6 +1109,30 @@ static int vmbus_acpi_remove(struct acpi_device *device) return 0; } +static void vmbus_reserve_fb(void) +{ + int size; + /* +* Make a claim for the frame buffer in the resource tree under the +* first node, which will be the one below 4GB. The length seems to +* be underreported, particularly in a Generation 1 VM. So start out +* reserving a larger area and make it smaller until it succeeds. +*/ + + if (screen_info.lfb_base) { + if (efi_enabled(EFI_BOOT)) + size = max_t(__u32, screen_info.lfb_size, 0x80); + else + size = max_t(__u32, screen_info.lfb_size, 0x400); + + for (; !fb_mmio && (size >= 0x10); size >>= 1) { + fb_mmio = __request_region(hyperv_mmio, + screen_info.lfb_base, size, + fb_mmio_name, 0); + } + } +} + /** * vmbus_allocate_mmio() - Pick a memory-mapped I/O range. * @new: If successful, supplied a pointer to the @@ -1261,8 +1294,10 @@ static int vmbus_acpi_add(struct acpi_device *device) if (ACPI_FAILURE(result)) continue; - if (hyperv_mmio) + if (hyperv_mmio) { + vmbus_reserve_fb(); break; + } } ret_val = 0; -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] PCI: hv: report resources release after stopping the bus
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Friday, April 29, 2016 2:39 AM > To: linux-...@vger.kernel.org > Cc: de...@linuxdriverproject.org; linux-ker...@vger.kernel.org; KY > Srinivasan ; Haiyang Zhang > ; Bjorn Helgaas ; Jake > Oshins > Subject: [PATCH] PCI: hv: report resources release after stopping the bus > > Kernel hang is observed when pci-hyperv module is release with device > drivers still attached. E.g. when I do 'rmmod pci_hyperv' with BCM5720 > device pass-through-ed (tg3 module) I see the following: > > NMI watchdog: BUG: soft lockup - CPU#1 stuck for 22s! [rmmod:2104] > ... > Call Trace: > [] tg3_read_mem+0x87/0x100 [tg3] > [] ? 0xa063f000 > [] tg3_poll_fw+0x85/0x150 [tg3] > [] tg3_chip_reset+0x357/0x8c0 [tg3] > [] tg3_halt+0x3b/0x190 [tg3] > [] tg3_stop+0x171/0x230 [tg3] > ... > [] tg3_remove_one+0x90/0x140 [tg3] > [] pci_device_remove+0x39/0xc0 > [] __device_release_driver+0xa1/0x160 > [] device_release_driver+0x23/0x30 > [] pci_stop_bus_device+0x8a/0xa0 > [] pci_stop_root_bus+0x36/0x60 > [] hv_pci_remove+0x238/0x260 [pci_hyperv] > > The problem seems to be that we report local resources release before > stopping the bus and removing devices from it and device drivers may > try to perform some operations with these resources on shutdown. Move > resources release report after we do pci_stop_root_bus(). > > Signed-off-by: Vitaly Kuznetsov Acked-by: Jake Oshins > --- > drivers/pci/host/pci-hyperv.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index f2559b6..c17e792 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -2268,11 +2268,6 @@ static int hv_pci_remove(struct hv_device *hdev) > > hbus = hv_get_drvdata(hdev); > > - ret = hv_send_resources_released(hdev); > - if (ret) > - dev_err(&hdev->device, > - "Couldn't send resources released packet(s)\n"); > - > memset(&pkt.teardown_packet, 0, sizeof(pkt.teardown_packet)); > init_completion(&comp_pkt.host_event); > pkt.teardown_packet.completion_func = hv_pci_generic_compl; > @@ -2295,6 +2290,11 @@ static int hv_pci_remove(struct hv_device *hdev) > pci_unlock_rescan_remove(); > } > > + ret = hv_send_resources_released(hdev); > + if (ret) > + dev_err(&hdev->device, > + "Couldn't send resources released packet(s)\n"); > + > vmbus_close(hdev->channel); > > /* Delete any children which might still exist. */ > -- > 2.5.5 This looks like the right fix to me. Thanks. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH [RFC]] PCI: hv: add explicit fencing to config space access
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Tuesday, May 3, 2016 5:22 AM > To: linux-...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; Bjorn > Helgaas ; Haiyang Zhang > ; KY Srinivasan ; Jake > Oshins > Subject: [PATCH [RFC]] PCI: hv: add explicit fencing to config space access > > I'm trying to pass-through Broadcom BCM5720 NIC (Dell Device 1f5b) on Dell > R720 server. Everything works fine when target VM has only one CPU, but > SMP guests reboot when NIC driver is trying to access PCI config space > (with hv_pcifront_read_config/hv_pcifront_write_config). The reboot > appears to be induced by the hypervisor and no crash is observed. Windows > event logs are not helpful at all ('Virtual machine ... has quit > unexpectedly'). The particular access point is always different and > putting debug between them (printk/mdelay/...) moves the issue further > away. The server model affects the issue as well: on Dell R420 I'm able to > pass-through BCM5720 NIC to SMP guests without issues. > > While I'm obviously failing to reveal the essence of the issue I was able > to come up with a (possible) solution: if explicit fencing is put to > hv_pcifront_read_config/hv_pcifront_write_config the issue goes away. The > essential minimum is rmb() at the end on _hv_pcifront_read_config() and > wmb() at the end of _hv_pcifront_write_config() but I'm not confident it > will be sufficient for all hardware. I suggest the following fencing: > 1) wmb()/mb() between choosing the function and writing to its space. > 2) mb() before releasing the spinlock in both _hv_pcifront_read_config()/ >_hv_pcifront_write_config to ensure that consecutive reads/writes to > the space won't get re-ordered as drivers may count on that. > Config space access is not supposed to be performance-critical so this > explicit fencing is not supposed to bring any slowdown. > > Signed-off-by: Vitaly Kuznetsov Acked-by: Jake Oshins > --- > drivers/pci/host/pci-hyperv.c | 14 ++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index c17e792..7e9b2de 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -553,6 +553,8 @@ static void _hv_pcifront_read_config(struct > hv_pci_dev *hpdev, int where, > spin_lock_irqsave(&hpdev->hbus->config_lock, flags); > /* Choose the function to be read. (See comment above) */ > writel(hpdev->desc.win_slot.slot, hpdev->hbus->cfg_addr); > + /* Make sure the function was chosen before we start > reading. */ > + mb(); > /* Read from that function's config space. */ > switch (size) { > case 1: > @@ -565,6 +567,11 @@ static void _hv_pcifront_read_config(struct > hv_pci_dev *hpdev, int where, > *val = readl(addr); > break; > } > + /* > + * Make sure the write was done before we release the > spinlock > + * allowing consecutive reads/writes. > + */ > + mb(); > spin_unlock_irqrestore(&hpdev->hbus->config_lock, flags); > } else { > dev_err(&hpdev->hbus->hdev->device, > @@ -592,6 +599,8 @@ static void _hv_pcifront_write_config(struct > hv_pci_dev *hpdev, int where, > spin_lock_irqsave(&hpdev->hbus->config_lock, flags); > /* Choose the function to be written. (See comment above) > */ > writel(hpdev->desc.win_slot.slot, hpdev->hbus->cfg_addr); > + /* Make sure the function was chosen before we start > writing. */ > + wmb(); > /* Write to that function's config space. */ > switch (size) { > case 1: > @@ -604,6 +613,11 @@ static void _hv_pcifront_write_config(struct > hv_pci_dev *hpdev, int where, > writel(val, addr); > break; > } > + /* > + * Make sure the write was done before we release the > spinlock > + * allowing consecutive reads/writes. > + */ > + mb(); > spin_unlock_irqrestore(&hpdev->hbus->config_lock, flags); > } else { > dev_err(&hpdev->hbus->hdev->device, > -- > 2.5.5 I'm honestly not as familiar with gcc or the Linux kernel as I should be. When I wrote this code, I assumed that the w
RE: [PATCH 1/2] PCI: hv: don't leak buffer in hv_pci_onchannelcallback()
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Monday, May 30, 2016 7:18 AM > To: linux-...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; Bjorn > Helgaas ; Haiyang Zhang > ; KY Srinivasan ; Jake > Oshins > Subject: [PATCH 1/2] PCI: hv: don't leak buffer in hv_pci_onchannelcallback() > > We don't free buffer on several code paths in hv_pci_onchannelcallback(), > put kfree() to the end of the function to fix the issue. Direct { kfree(); > return; } can now be replaced with a simple 'break'; > > Signed-off-by: Vitaly Kuznetsov Acked-by: Jake Oshins > --- > drivers/pci/host/pci-hyperv.c | 11 +-- > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 7e9b2de..a68ec49 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1661,10 +1661,8 @@ static void hv_pci_onchannelcallback(void > *context) >* All incoming packets must be at least as large as a >* response. >*/ > - if (bytes_recvd <= sizeof(struct pci_response)) { > - kfree(buffer); > - return; > - } > + if (bytes_recvd <= sizeof(struct pci_response)) > + break; > desc = (struct vmpacket_descriptor *)buffer; > > switch (desc->type) { > @@ -1679,8 +1677,7 @@ static void hv_pci_onchannelcallback(void > *context) > comp_packet->completion_func(comp_packet- > >compl_ctxt, >response, >bytes_recvd); > - kfree(buffer); > - return; > + break; > > case VM_PKT_DATA_INBAND: > > @@ -1729,6 +1726,8 @@ static void hv_pci_onchannelcallback(void > *context) > } > break; > } > + > + kfree(buffer); > } > > /** > -- > 2.5.5 This is a good fix. Thanks. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/2] PCI: hv: handle all pending messages in hv_pci_onchannelcallback()
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Monday, May 30, 2016 7:18 AM > To: linux-...@vger.kernel.org > Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; Bjorn > Helgaas ; Haiyang Zhang > ; KY Srinivasan ; Jake > Oshins > Subject: [PATCH 2/2] PCI: hv: handle all pending messages in > hv_pci_onchannelcallback() > > When we have an interrupt from host we have a bit set in event page > indicating there are messages for the particular channel. We need to read > them all as we won't get signaled for what was on the queue before we > cleared the bit in vmbus_on_event(). This applies to all Hyper-V drivers > and the pass-through driver should do the same. > I did non meet any bugs, the issue was found by code inspection. We don't > have many events going through hv_pci_onchannelcallback(), this explains > why nobody reported the issue before. > > While on it, fix handling non-zero vmbus_recvpacket_raw() return values by > dropping out. If the return value is not zero it is wrong to inspect > buffer or bytes_recvd as these may contain invalid data. > > Signed-off-by: Vitaly Kuznetsov Acked-by: Jake Oshins > --- > drivers/pci/host/pci-hyperv.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index a68ec49..7de341d 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1657,12 +1657,16 @@ static void hv_pci_onchannelcallback(void > *context) > continue; > } > > + /* Zero length indicates there are no more packets. */ > + if (ret || !bytes_recvd) > + break; > + > /* >* All incoming packets must be at least as large as a >* response. >*/ > if (bytes_recvd <= sizeof(struct pci_response)) > - break; > + continue; > desc = (struct vmpacket_descriptor *)buffer; > > switch (desc->type) { > @@ -1724,7 +1728,6 @@ static void hv_pci_onchannelcallback(void > *context) > desc->type, req_id, bytes_recvd); > break; > } > - break; > } > > kfree(buffer); > -- > 2.5.5 This is good, too. Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
> -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: Wednesday, February 3, 2016 10:25 AM > To: Jake Oshins > Cc: gre...@linuxfoundation.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang > ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Subject: Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata > > Hi Jake, > > On Tue, Feb 02, 2016 at 05:41:41PM +, ja...@microsoft.com wrote: > > From: Jake Oshins > > > > This patch adds an fwnode_handle to struct pci_sysdata, which is used > > by the next patch in the series when trying to locate an IRQ domain > > associated with a root PCI bus. > > > > Signed-off-by: Jake Oshins > > --- > > arch/x86/include/asm/pci.h | 15 +++ > > drivers/pci/probe.c| 1 + > > include/linux/pci.h| 4 > > 3 files changed, 20 insertions(+) > > > > diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h > > index 4625943..6fc3c7c 100644 > > --- a/arch/x86/include/asm/pci.h > > +++ b/arch/x86/include/asm/pci.h > > @@ -20,6 +20,9 @@ struct pci_sysdata { #ifdef CONFIG_X86_64 > > void*iommu; /* IOMMU private data */ > > #endif > > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > > + void*fwnode;/* IRQ domain for MSI assignment */ > > +#endif > > }; > > > > extern int pci_routeirq; > > @@ -32,6 +35,7 @@ extern int noioapicreroute; static inline int > > pci_domain_nr(struct pci_bus *bus) { > > struct pci_sysdata *sd = bus->sysdata; > > + > > return sd->domain; > > } > > > > @@ -41,6 +45,17 @@ static inline int pci_proc_domain(struct pci_bus > > *bus) } #endif > > > > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > > +static inline void *_pci_root_bus_fwnode(struct pci_bus *bus) { > > + struct pci_sysdata *sd = bus->sysdata; > > + > > + return sd->fwnode; > > +} > > + > > +#define pci_root_bus_fwnode_pci_root_bus_fwnode > > +#endif > > + > > /* Can be used to override the logic in pci_scan_bus for skipping > > already-configured bus numbers - to be used for buggy BIOSes > > or architectures with incomplete PCI setup by the loader */ diff > > --git a/drivers/pci/probe.c b/drivers/pci/probe.c index > > 6d7ab9b..b207e74 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -15,6 +15,7 @@ > > #include > > #include > > #include > > +#include > > You're not adding a use of anything in irqdomain.h. It looks like this hunk > should be moved to the second patch. Wil do. > > > #include > > #include "pci.h" > > > > diff --git a/include/linux/pci.h b/include/linux/pci.h index > > 27df4a6..cd05a8e 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -1515,6 +1515,10 @@ static inline int pci_get_new_domain_nr(void) { > > return -ENOSYS; } > > > > #include > > > > +#ifndef pci_root_bus_fwnode > > +#define pci_root_bus_fwnode(bus) ((void)(bus), NULL) > > Huh, interesting. This is new for me; I guess the idea is that we at least > evaluate "bus" even when pci_root_bus_fwnode isn't defined, so the > compiler can catch egregious errors? > This was a suggestion by Mark Zyngier. It made the non-x86 architectures build benignly. If you'd like it done differently, I'm open to suggestion. Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH RESEND 3/3] PCI: hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: Wednesday, February 3, 2016 1:29 PM > To: Jake Oshins > Cc: gre...@linuxfoundation.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang > ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Subject: Re: [PATCH RESEND 3/3] PCI: hv: New paravirtual PCI front-end for > Hyper-V VMs > > Hi Jake, > > On Tue, Feb 02, 2016 at 05:41:43PM +0000, ja...@microsoft.com wrote: > > From: Jake Oshins > > > > This patch introduces a new driver which exposes a root PCI bus whenever > a > > PCI Express device is passed through to a guest VM under Hyper-V. The > > device can be single- or multi-function. The interrupts for the devices > > are managed by an IRQ domain, implemented within the driver. [lots of stuff snipped out] > > +/** > > + * hv_pci_free_bridge_windows() - Release memory regions for the > > + * bus > > + * @hbus: Root PCI bus, as understood by this driver > > + */ > > +static void hv_pci_free_bridge_windows(struct hv_pcibus_device *hbus) > > +{ > > + if (hbus->low_mmio_space && hbus->low_mmio_res) { > > + hbus->low_mmio_res->flags |= IORESOURCE_BUSY; > > Drivers should not normally set or clear IORESOURCE_BUSY themselves. > > > + release_mem_region(hbus->low_mmio_res->start, > > + resource_size(hbus->low_mmio_res)); > > Usually there's a request_mem_region() to correspond with a > release_mem_region(), and that takes care of IORESOURCE_BUSY. > > What's unique about this driver, and can you make it less unique? > Thanks for the detailed review. I'll make all the changes that you're suggesting and resend. As for the comment above, and all the others related to IORESOURCE_BUSY, this amounts to making the resource claim look like a bridge window, so that callers of request_mem_region() in the drivers for the child devices actually can succeed. There's no function in the kernel today that amounts to "request_mem_region_for_bridge_window()" or at least none that I understood. The current plug and play code in Linux is pretty hard coded for various bus types, i.e. ACPI and PCI. I took a tack at one point where I tried to offer changes to it, so that it understood the concept of "grandchild of ACPI or PCI" which is what would make this code simpler. Rafael Wysocki basically told me that the pnp layer is deprecated and that new changes to it wouldn't be entertained, and that I should find some way of using what exists. This was the result. If you have another suggestion, I'm happy to try it. In any case, I'll explain what's happening more thoroughly in comments. Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata
> -Original Message- > From: Marc Zyngier [mailto:marc.zyng...@arm.com] > Sent: Wednesday, February 3, 2016 10:57 AM > To: Bjorn Helgaas ; Jake Oshins > > Cc: gre...@linuxfoundation.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; Haiyang Zhang > ; bhelg...@google.com; linux- > p...@vger.kernel.org > Subject: Re: [PATCH RESEND 1/3] PCI: Add fwnode_handle to pci_sysdata > > On 03/02/16 18:51, Bjorn Helgaas wrote: > > On Wed, Feb 03, 2016 at 06:32:20PM +, Jake Oshins wrote: > >>> -Original Message- > >>> From: Bjorn Helgaas [mailto:helg...@kernel.org] > >>> Sent: Wednesday, February 3, 2016 10:25 AM > >>> To: Jake Oshins [snip] > >>>> > >>>> +#ifndef pci_root_bus_fwnode > >>>> +#define pci_root_bus_fwnode(bus)((void)(bus), NULL) > >>> > >>> Huh, interesting. This is new for me; I guess the idea is that we at > >>> least > >>> evaluate "bus" even when pci_root_bus_fwnode isn't defined, so the > >>> compiler can catch egregious errors? > >>> > >> > >> This was a suggestion by Mark Zyngier. It made the non-x86 architectures > build benignly. If you'd like it done differently, I'm open to suggestion. > > I don't remember suggesting the use of the comma operator, but just to > check that pci_root_bus_fwnode wasn't previously defined. > > > Something like "#define pci_root_bus_fwnode(bus) NULL" would be > > typical. What I'm curious about is the use of the comma operator. > > I'm not opposed to it; I'm just trying to understand why it makes a > > difference. > > I guess it flags the variable as used, and prevents an overly sensitive > compiler from being loud and obnoxious... Just a guess though. > > M. > -- I was just copying the form that was in the code that Marc pointed out, somewhere else. I have no particular attachment to this construct. Honestly, since I'm still relatively new to the Linux codebase, I just assumed that was a common form that I should follow suit on. If you tell me that the simpler NULL is more typical, I prefer that. I'll simplify this when I resend the patch series. Thanks, Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH RESEND v2 3/3] PCI: hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: Tuesday, February 16, 2016 8:46 AM > To: Jake Oshins > Cc: bhelg...@google.com; linux-...@vger.kernel.org; > gre...@linuxfoundation.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@redhat.com; t...@linutronix.de; Haiyang > Zhang ; marc.zyng...@arm.com; Hadden > Hoppert > Subject: Re: [PATCH RESEND v2 3/3] PCI: hv: New paravirtual PCI front-end > for Hyper-V VMs > > Hi Jake, > > Looks good to me overall; I marked a few nits below. > > The only real question I have is about domain number allocation. See > the note below. > [snip] > > + > > + /* > > +* The PCI bus "domain" is what is called "segment" in > > +* ACPI and other specs. Pull it from the instance ID, > > +* to get something unique. Bytes 8 and 9 are what is used > > +* in Windows guests, so do the same thing for consistency. > > +*/ > > + > > + hbus->sysdata.domain = hdev->dev_instance.b[9] | > > + hdev->dev_instance.b[8] << 8; > > How do we know this is unique? We don't have any idea what the > platform will put in _SEG, so I think there's a potential conflict > here. The Intel VMD driver (arch/x86/pci/vmd.c) has a similar > problem, and it looks for unused domain numbers starting at 0x1 > (see vmd_find_free_domain()). > Bjorn, thank you for your very thorough reviews. I deeply appreciate it. I checked the Hyper-V code and it currently does guarantee that these values are unique. When I resend the series, I'll add a comment to that effect. I'll also add a comment to Hyper-V that says that it has to stay that way. I'll resend shortly. Thanks again, Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH RESEND v2 3/3] PCI: hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: Bjorn Helgaas [mailto:helg...@kernel.org] > Sent: Tuesday, February 16, 2016 2:44 PM > To: Jake Oshins > Cc: bhelg...@google.com; linux-...@vger.kernel.org; > gre...@linuxfoundation.org; KY Srinivasan ; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@redhat.com; t...@linutronix.de; Haiyang > Zhang ; marc.zyng...@arm.com; Hadden > Hoppert > Subject: Re: [PATCH RESEND v2 3/3] PCI: hv: New paravirtual PCI front-end > for Hyper-V VMs > > On Tue, Feb 16, 2016 at 09:46:55PM +, Jake Oshins wrote: > > > -Original Message- > > > From: Bjorn Helgaas [mailto:helg...@kernel.org] > > > Sent: Tuesday, February 16, 2016 8:46 AM > > > To: Jake Oshins > > > Cc: bhelg...@google.com; linux-...@vger.kernel.org; > > > gre...@linuxfoundation.org; KY Srinivasan ; > > > linux- ker...@vger.kernel.org; de...@linuxdriverproject.org; > > > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com; > > > t...@linutronix.de; Haiyang Zhang ; > > > marc.zyng...@arm.com; Hadden Hoppert > > > Subject: Re: [PATCH RESEND v2 3/3] PCI: hv: New paravirtual PCI > > > front-end for Hyper-V VMs > > > > > > Hi Jake, > > > > > > Looks good to me overall; I marked a few nits below. > > > > > > The only real question I have is about domain number allocation. > > > See the note below. > > > > > [snip] > > > > + > > > > + /* > > > > +* The PCI bus "domain" is what is called "segment" in > > > > +* ACPI and other specs. Pull it from the instance ID, > > > > +* to get something unique. Bytes 8 and 9 are what is used > > > > +* in Windows guests, so do the same thing for consistency. > > > > +*/ > > > > + > > > > + hbus->sysdata.domain = hdev->dev_instance.b[9] | > > > > + hdev->dev_instance.b[8] << 8; > > > > > > How do we know this is unique? We don't have any idea what the > > > platform will put in _SEG, so I think there's a potential conflict > > > here. The Intel VMD driver (arch/x86/pci/vmd.c) has a similar > > > problem, and it looks for unused domain numbers starting at 0x1 > > > (see vmd_find_free_domain()). > > > > > > > Bjorn, thank you for your very thorough reviews. I deeply appreciate it. I > checked the Hyper-V code and it currently does guarantee that these values > are unique. When I resend the series, I'll add a comment to that effect. > I'll > also add a comment to Hyper-V that says that it has to stay that way. > > I'm not familiar with how Hyper-V works, but I guess you're saying that > Hyper-V controls what the guest sees via _SEG as well as what it sees via the > instance ID. > > Bjorn Yes, exactly. To the point, the Hyper-V Generation 1 VM (which is an emulation of a 440BX/PIIX4 platform) has one root PCI bus with no _SEG object at all, which implies a value of zero. The Hyper-V Generation 2 VM (which doesn't emulate any existing machine) doesn't have any root PCI bus at all until a device is passed through and this driver is loaded. -- Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 5/5] hv: Track allocations of children of hv_vmbus in private resource tree
> -Original Message- > From: KY Srinivasan > Sent: Friday, February 26, 2016 5:09 PM > To: Jake Oshins ; linux-...@vger.kernel.org; > gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; Haiyang Zhang ; Hadden > Hoppert > Cc: Jake Oshins > Subject: RE: [PATCH 5/5] hv: Track allocations of children of hv_vmbus in > private resource tree > > > -Original Message- > > From: ja...@microsoft.com [mailto:ja...@microsoft.com] > > Sent: Wednesday, February 24, 2016 1:24 PM > > To: linux-...@vger.kernel.org; gre...@linuxfoundation.org; KY Srinivasan > > ; linux-ker...@vger.kernel.org; > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > > vkuzn...@redhat.com; Haiyang Zhang ; > Hadden > > Hoppert > > Cc: Jake Oshins > > Subject: [PATCH 5/5] hv: Track allocations of children of hv_vmbus in > private > > resource tree > > > > From: Jake Oshins > > > > This patch changes vmbus_allocate_mmio() and vmbus_free_mmio() so > > that when child paravirtual devices allocate memory-mapped I/O > > space, they allocate it privately from a resource tree pointed > > at by hyperv_mmio and also by the public resource tree > > iomem_resource. This allows the region to be marked as "busy" > > in the private tree, but a "bridge window" in the public tree, > > guaranteeing that no two bridge windows will overlap each other > > but while also allowing the PCI device children of the bridge > > windows to overlap that window. > > > > One might conclude that this belongs in the pnp layer, rather > > than in this driver. Rafael Wysocki, the maintainter of the > > pnp layer, has previously asked that we not modify the pnp layer > > as it is considered deprecated. This patch is thus essentially > > a workaround. > > > > Signed-off-by: Jake Oshins > > --- > > drivers/hv/vmbus_drv.c | 22 +- > > 1 file changed, 21 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c > > index b090548..2a7eb3f 100644 > > --- a/drivers/hv/vmbus_drv.c > > +++ b/drivers/hv/vmbus_drv.c > > @@ -1169,7 +1169,7 @@ int vmbus_allocate_mmio(struct resource > **new, > > struct hv_device *device_obj, > > resource_size_t size, resource_size_t align, > > bool fb_overlap_ok) > > { > > - struct resource *iter; > > + struct resource *iter, *shadow; > > resource_size_t range_min, range_max, start, local_min, local_max; > > const char *dev_n = dev_name(&device_obj->device); > > u32 fb_end = screen_info.lfb_base + (screen_info.lfb_size << 1); > > @@ -1211,12 +1211,22 @@ int vmbus_allocate_mmio(struct resource > > **new, struct hv_device *device_obj, > > > > start = (local_min + align - 1) & ~(align - 1); > > for (; start + size - 1 <= local_max; start += align) { > > + shadow = __request_region(iter, start, > > + size, > > + NULL, > > + IORESOURCE_BUSY); > > + if (!shadow) > > + continue; > > + > > *new = > > request_mem_region_exclusive(start, size, > > dev_n); > > if (*new) { > > + shadow->name = (char*)*new; > > Why are you not correctly setting the name field in the shadow structure? > > Regards, > > K. Y Nothing looks at the name fields in the shadow resource tree. So it seemed like that pointer could point to anything. I figured by making it point to the resource claim from the iomem_resource that might be useful in debugging someday. If you'd rather see something different here, it doesn't make much difference to me. Thanks for the review, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [patch] PCI: hv: potential use after free
> -Original Message- > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > Sent: Saturday, February 27, 2016 2:44 AM > To: KY Srinivasan ; Jake Oshins > > Cc: Haiyang Zhang ; Bjorn Helgaas > ; de...@linuxdriverproject.org; linux- > p...@vger.kernel.org; linux-ker...@vger.kernel.org; kernel- > janit...@vger.kernel.org > Subject: [patch] PCI: hv: potential use after free > > If we throw away the very last item on the list, then we could end up > with a use after free of "dr". > > Fixes: 15ca17645f19 ('PCI: hv: Add paravirtual PCI front-end for Microsoft > Hyper-V VMs') > Signed-off-by: Dan Carpenter > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 9391dee..9b66ffe 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1397,6 +1397,7 @@ static void pci_devices_present_work(struct > work_struct *work) > /* Throw this away if the list still has stuff in it. */ > if (!list_empty(&hbus->dr_list)) { > kfree(dr); > + dr = NULL; > continue; > } > } Thanks for looking at this. I do truly appreciate it. But the code here removes dr from the list and then, before freeing it, checks to see that it was not the last entry in the list. The list lock is still held and the list is not empty even after removing dr from it. (I suspect that you're going to tell me that I'm missing something here. Please do. I'll appreciate it even more.) Thanks, Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 6/6] drivers:pci:hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: Paul Bolle [mailto:pebo...@tiscali.nl] > Sent: Friday, June 12, 2015 1:44 AM > To: Jake Oshins > Cc: gre...@linuxfoundation.org; KY Srinivasan; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@redhat.com; linux-...@vger.kernel.org; > bhelg...@google.com; Mike Ebersol; Haiyang Zhang > Subject: Re: [PATCH 6/6] drivers:pci:hv: New paravirtual PCI front-end for > Hyper-V VMs > > Greg has already asked you to resend. So here follow a few remarks to > take into account for that resend. > Thank you. I'll fix everything you've mentioned before resending. I do have one more question, below. > > > +EXPORT_SYMBOL(hv_read_config_block); > > > +EXPORT_SYMBOL(hv_write_config_block); > > > +EXPORT_SYMBOL(hv_register_block_invalidate); > > I couldn't spot any users of these exports. Actually, I couldn't even > spot any users of these three functions. Why were they added? > This driver is intended to support both full PCI Express device pass through and also be the basis for SR-IOV networking on top of Hyper-V. These functions would allow somebody trying to make their NIC driver work on top of Hyper-V to exchange messages with their back-end Windows driver. My question is this. How does somebody delivering a platform usually work with the Linux community to deliver enablement code like this? I'm trying to work in the open, and go upstream early (or at least I think that understand what these things mean.) If the community doesn't want functions that have no callers (and I understand that, too) then how should I provide them to the NIC vendors? Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 4/6] drivers:hv: Export a function that maps Linux proc num onto Hyper-V proc num
> -Original Message- > From: Greg KH [mailto:gre...@linuxfoundation.org] > Sent: Monday, June 15, 2015 12:52 PM > To: Jake Oshins > Cc: KY Srinivasan; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; bhelg...@google.com; linux-...@vger.kernel.org; > pebo...@tiscali.nl; Haiyang Zhang; Mike Ebersol > Subject: Re: [PATCH v2 4/6] drivers:hv: Export a function that maps Linux > proc num onto Hyper-V proc num > > > +/** > > + * This function returns the mapping between the Linux processor > > + * number and > > + * the hypervisor's virtual processor number, useful in making > > + * hypercalls and such that talk about specific processors. > > + * > > + * @param procnum - in Linux terms > > What kind of comment structure is this? It's not kernel doc :( > > And "procnum" sounds like "process number", why not "cpu_number"? Thanks. I'll wait a little while to see if any other feedback comes in and then I'll fix these and resend. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3 1/6] hv: Modify vmbus to search for all MMIO ranges available
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Wednesday, June 17, 2015 10:28 AM > To: Jake Oshins > Cc: gre...@linuxfoundation.org; KY Srinivasan; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; Haiyang Zhang; Mike Ebersol > Subject: Re: [PATCH v3 1/6] hv: Modify vmbus to search for all MMIO ranges > available > > } > > > > @@ -1047,6 +1121,7 @@ static struct acpi_driver vmbus_acpi_driver = { > > .ids = vmbus_acpi_device_ids, > > .ops = { > > .add = vmbus_acpi_add, > > + .remove = vmbus_acpi_remove, > > This will probably need rebasing on top of current char-misc-next tree > as we already have commit e4ecb41c: "Drivers: hv: vmbus: introduce > vmbus_acpi_remove" there. > Thanks. Please educate me since I'm new around here. What should I do in response to this message? Wait for this tree to be pulled into the mainline and resend after rebasing? Something more proactive? -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3 6/6] drivers:pci:hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > Sent: Monday, June 22, 2015 7:14 AM > To: Jake Oshins > Cc: gre...@linuxfoundation.org; KY Srinivasan; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@redhat.com; Haiyang Zhang; Mike Ebersol > Subject: Re: [PATCH v3 6/6] drivers:pci:hv: New paravirtual PCI front-end for > Hyper-V VMs > > Small bug fixes and cleanups. Please re-indent > pci_devices_present_work() especially. > > > > regards, > dan carpenter Thanks for your review. I'll respond to all of it and resend. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 4/4] drivers:pci:hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: Thomas Gleixner [mailto:t...@linutronix.de] > Sent: Sunday, August 2, 2015 1:47 AM > To: Jake Oshins > Cc: gre...@linuxfoundation.org; KY Srinivasan ; LKML > ; de...@linuxdriverproject.org; > o...@aepfle.de; a...@canonical.com; vkuzn...@redhat.com; linux- > p...@vger.kernel.org; bhelg...@google.com; x...@kernel.org; Jiang Liu > > Subject: Re: [PATCH 4/4] drivers:pci:hv: New paravirtual PCI front-end for > Hyper-V VMs > > > +static int hv_msi_domain_ops_prepare(struct irq_domain *domain, > > +struct device *dev, int nvec, > > +msi_alloc_info_t *arg) > > +{ > > + struct pci_dev *pdev = to_pci_dev(dev); > > + struct msi_desc *desc = first_pci_msi_entry(pdev); > > + > > + memset(arg, 0, sizeof(*arg)); > > + arg->msi_dev = pdev; > > + if (desc->msi_attrib.is_msix) { > > + arg->type = X86_IRQ_ALLOC_TYPE_MSIX; > > + } else { > > + arg->type = X86_IRQ_ALLOC_TYPE_MSI; > > + arg->flags |= X86_IRQ_ALLOC_CONTIGUOUS_VECTORS; > > + } > > + > > + return 0; > > +} > > Pretty much a copy of the x86 code, right? > > I wonder whether this MSI infrastructure code would be better > seperated out and moved to arch/x86/hyperv/msi.c or > arch/x86/kernel/apic/hvmsi.c. It's small enough to be built in. So all > you'd need to export is hv_pcie_init_irq_domain and > hv_pcie_free_irq_domain. > > Thanks, > > tglx Thanks for your review. I'll respond to all of your feedback and resend. I do have a question about your last point, though. If I build this into the kernel, it will need to depend on sending and receiving messages through the hv_vmbus driver, which isn't built in. It seemed like the indirection and glue code necessary to make that work would be almost as big as this entire implementation (which admittedly isn't very big.) If you prefer that, I'll do it. But it would make more sense to me to refactor this a bit more so that functions like the one above are exported rather than putting hv_pcie_init_irq_domain and hv_pcie_free_irq_domain into the kernel itself. Would that work? Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [bug report] PCI: hv: Add paravirtual PCI front-end for Microsoft Hyper-V VMs
> -Original Message- > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > Sent: Monday, February 6, 2017 11:12 PM > To: Jake Oshins > Cc: de...@linuxdriverproject.org; linux-...@vger.kernel.org > Subject: [bug report] PCI: hv: Add paravirtual PCI front-end for Microsoft > Hyper-V VMs > > [ No idea why I haven never sent this email before. I was just going > through all the use after free warnings again today and noticed it. ] > > Hello Jake Oshins, > > The patch 4daace0d8ce8: "PCI: hv: Add paravirtual PCI front-end for > Microsoft Hyper-V VMs" from Feb 16, 2016, leads to the following > static checker warning: > > drivers/pci/host/pci-hyperv.c:1441 pci_devices_present_work() > error: dereferencing freed memory 'dr' > > drivers/pci/host/pci-hyperv.c > 1410 /* Pull this off the queue and process it if it was the last > one. */ > 1411 spin_lock_irqsave(&hbus->device_list_lock, flags); > 1412 while (!list_empty(&hbus->dr_list)) { > 1413 dr = list_first_entry(&hbus->dr_list, struct > hv_dr_state, > 1414list_entry); > 1415 list_del(&dr->list_entry); > 1416 > 1417 /* Throw this away if the list still has stuff in it. > */ > 1418 if (!list_empty(&hbus->dr_list)) { > 1419 kfree(dr); > ^ > We free "dr". Presumably we should set dr = NULL here? > > 1420 continue; > 1421 } > 1422 } > 1423 spin_unlock_irqrestore(&hbus->device_list_lock, flags); > 1424 > 1425 if (!dr) { > 1426 up(&hbus->enum_sem); > 1427 put_hvpcibus(hbus); > 1428 return; > 1429 } > 1430 > 1431 /* First, mark all existing children as reported missing. */ > 1432 spin_lock_irqsave(&hbus->device_list_lock, flags); > 1433 list_for_each(iter, &hbus->children) { > 1434 hpdev = container_of(iter, struct hv_pci_dev, > 1435 list_entry); > 1436 hpdev->reported_missing = true; > 1437 } > 1438 spin_unlock_irqrestore(&hbus->device_list_lock, flags); > 1439 > 1440 /* Next, add back any reported devices. */ > 1441 for (child_no = 0; child_no < dr->device_count; child_no++) { > > Use after free. > > 1442 found = false; > 1443 new_desc = &dr->func[child_no]; > 1444 > 1445 spin_lock_irqsave(&hbus->device_list_lock, flags); > > > regards, > dan carpenter I'm pretty sure that this is a false positive. It only frees the struct if there is another entry in the list, and then it immediately overwrites the pointer with the next entry in the list. What's the right move here? Should we send a patch that nulls the pointer just to make a static analysis hit go away? It's not a hot path. Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] drivers:hv:vmbus Allow for more than one MMIO range for children.
Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 85 + drivers/video/fbdev/hyperv_fb.c | 2 +- include/linux/hyperv.h | 2 +- 3 files changed, 72 insertions(+), 17 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 4d6b269..ba5b7a1 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -43,10 +43,7 @@ static struct tasklet_struct msg_dpc; static struct completion probe_event; static int irq; -struct resource hyperv_mmio = { - .name = "hyperv mmio", - .flags = IORESOURCE_MEM, -}; +struct resource *hyperv_mmio; EXPORT_SYMBOL_GPL(hyperv_mmio); static int vmbus_exists(void) @@ -849,23 +846,74 @@ void vmbus_device_unregister(struct hv_device *device_obj) /* - * VMBUS is an acpi enumerated device. Get the the information we - * need from DSDT. + * VMBUS is an acpi enumerated device. Get the + * information we need from DSDT. */ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) { + resource_size_t start = 0; + resource_size_t end = 0; + struct resource *new_res; + struct resource **old_res = &hyperv_mmio; + switch (res->type) { case ACPI_RESOURCE_TYPE_IRQ: irq = res->data.irq.interrupts[0]; + return AE_OK; + + /* +* "Address" descriptors are for bus windows. Ignore +* "memory" descriptors, which are for registers on +* devices. +*/ + case ACPI_RESOURCE_TYPE_ADDRESS32: + start = res->data.address32.minimum; + end = res->data.address32.maximum; break; case ACPI_RESOURCE_TYPE_ADDRESS64: - hyperv_mmio.start = res->data.address64.minimum; - hyperv_mmio.end = res->data.address64.maximum; + start = res->data.address64.minimum; + end = res->data.address64.maximum; break; + + default: + /* Unused resource type */ + return AE_OK; } + /* +* Ignore ranges that are below 1MB, as they're not +* necessary or useful here. +*/ + if (end < 0x10) + return AE_OK; + + new_res = kzalloc(sizeof(*new_res), GFP_ATOMIC); + if (!new_res) + return AE_NO_MEMORY; + + new_res->name = "hyperv mmio"; + new_res->flags = IORESOURCE_MEM; + new_res->start = start; + new_res->end = end; + + do { + if (!*old_res) { + *old_res = new_res; + break; + } + + if ((*old_res)->start > new_res->end) { + new_res->sibling = *old_res; + *old_res = new_res; + break; + } + + old_res = &(*old_res)->sibling; + + } while (1); + return AE_OK; } @@ -873,6 +921,7 @@ static int vmbus_acpi_add(struct acpi_device *device) { acpi_status result; int ret_val = -ENODEV; + struct acpi_device *ancestor; hv_acpi_dev = device; @@ -882,18 +931,22 @@ static int vmbus_acpi_add(struct acpi_device *device) if (ACPI_FAILURE(result)) goto acpi_walk_err; /* -* The parent of the vmbus acpi device (Gen2 firmware) is the VMOD that -* has the mmio ranges. Get that. +* Some ancestor of the vmbus acpi device (Gen1 or Gen2 +* firmware) is the VMOD that has the mmio ranges. Get that. */ - if (device->parent) { - result = acpi_walk_resources(device->parent->handle, + for (ancestor = device->parent; + ancestor; + ancestor = ancestor->parent) { + result = acpi_walk_resources(ancestor->handle, METHOD_NAME__CRS, vmbus_walk_resources, NULL); if (ACPI_FAILURE(result)) - goto acpi_walk_err; - if (hyperv_mmio.start && hyperv_mmio.end) - request_resource(&iomem_resource, &hyperv_mmio); + continue; + if (hyperv_mmio) { + request_resource(&iomem_resource, hyperv_mmio); + break; + } } ret_val = 0; @@ -962,6 +1015,8 @@ static void __exit vmbus_exit(void) hv_remove_vmbus_irq(); vmbus_free_channels(); bus_unregister(&hv_bus); + if (hyperv_mmio) + release_resource(hyperv_mmio); hv_cleanup(); acpi_bus_unregister_driver(&vmbus_acpi_driver); } diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_
[PATCH 1/1] drivers:hv:vmbus Allow for use of all MMIO ranges
This set of changes finds the _CRS object in the ACPI namespace that contains memory address space descriptors, intended to convey to VMBus which ranges of memory-mapped I/O space are available for child devices, and then builds a resource list that contains all those ranges. Without this change, only some of the memory-mapped I/O space will be available for child devices, and only in some virtual BIOS configurations (Generation 2 VMs). Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 97 ++--- drivers/video/fbdev/hyperv_fb.c | 2 +- include/linux/hyperv.h | 2 +- 3 files changed, 84 insertions(+), 17 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 4d6b269..b4c4b92 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -43,10 +43,7 @@ static struct tasklet_struct msg_dpc; static struct completion probe_event; static int irq; -struct resource hyperv_mmio = { - .name = "hyperv mmio", - .flags = IORESOURCE_MEM, -}; +struct resource *hyperv_mmio; EXPORT_SYMBOL_GPL(hyperv_mmio); static int vmbus_exists(void) @@ -849,30 +846,96 @@ void vmbus_device_unregister(struct hv_device *device_obj) /* - * VMBUS is an acpi enumerated device. Get the the information we - * need from DSDT. + * VMBUS is an acpi enumerated device. Get the + * information we need from DSDT. */ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) { + resource_size_t start = 0; + resource_size_t end = 0; + struct resource *new_res; + struct resource **old_res = &hyperv_mmio; + switch (res->type) { case ACPI_RESOURCE_TYPE_IRQ: irq = res->data.irq.interrupts[0]; + return AE_OK; + + /* +* "Address" descriptors are for bus windows. Ignore +* "memory" descriptors, which are for registers on +* devices. +*/ + case ACPI_RESOURCE_TYPE_ADDRESS32: + start = res->data.address32.minimum; + end = res->data.address32.maximum; break; case ACPI_RESOURCE_TYPE_ADDRESS64: - hyperv_mmio.start = res->data.address64.minimum; - hyperv_mmio.end = res->data.address64.maximum; + start = res->data.address64.minimum; + end = res->data.address64.maximum; break; + + default: + /* Unused resource type */ + return AE_OK; } + /* +* Ignore ranges that are below 1MB, as they're not +* necessary or useful here. +*/ + if (end < 0x10) + return AE_OK; + + new_res = kzalloc(sizeof(*new_res), GFP_ATOMIC); + if (!new_res) + return AE_NO_MEMORY; + + new_res->name = "hyperv mmio"; + new_res->flags = IORESOURCE_MEM; + new_res->start = start; + new_res->end = end; + + do { + if (!*old_res) { + *old_res = new_res; + break; + } + + if ((*old_res)->start > new_res->end) { + new_res->sibling = *old_res; + *old_res = new_res; + break; + } + + old_res = &(*old_res)->sibling; + + } while (1); + return AE_OK; } +static void vmbus_release_resources(void) +{ + struct resource *cur_res; + struct resource *next_res; + + if (hyperv_mmio) { + release_resource(hyperv_mmio); + for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) { + next_res = cur_res->sibling; + kfree(cur_res); + } + } +} + static int vmbus_acpi_add(struct acpi_device *device) { acpi_status result; int ret_val = -ENODEV; + struct acpi_device *ancestor; hv_acpi_dev = device; @@ -882,23 +945,26 @@ static int vmbus_acpi_add(struct acpi_device *device) if (ACPI_FAILURE(result)) goto acpi_walk_err; /* -* The parent of the vmbus acpi device (Gen2 firmware) is the VMOD that -* has the mmio ranges. Get that. +* Some ancestor of the vmbus acpi device (Gen1 or Gen2 +* firmware) is the VMOD that has the mmio ranges. Get that. */ - if (device->parent) { - result = acpi_walk_resources(device->parent->handle, + for (ancestor = device->parent; ancestor; ancestor = ancestor->parent) { + result = acpi_walk_resources(ancestor->handle, METHOD_NAME__CRS, vmbus_walk_resources, NULL); if (ACPI_FAILURE(result)) -
[PATCH v2 1/1] drivers:hv:vmbus drivers:hv:vmbus Allow for more than one MMIO range for children
This set of changes finds the _CRS object in the ACPI namespace that contains memory address space descriptors, intended to convey to VMBus which ranges of memory-mapped I/O space are available for child devices, and then builds a resource list that contains all those ranges. Without this change, only some of the memory-mapped I/O space will be available for child devices, and only in some virtual BIOS configurations (Generation 2 VMs). This patch has been updated with feedback from Vitaly Kuznetsov. Cleanup is now driven by the acpi remove callback function. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 99 +-- drivers/video/fbdev/hyperv_fb.c |2 +- include/linux/hyperv.h |2 +- 3 files changed, 86 insertions(+), 17 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 4d6b269..ed618ac 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -43,10 +43,7 @@ static struct tasklet_struct msg_dpc; static struct completion probe_event; static int irq; -struct resource hyperv_mmio = { - .name = "hyperv mmio", - .flags = IORESOURCE_MEM, -}; +struct resource *hyperv_mmio; EXPORT_SYMBOL_GPL(hyperv_mmio); static int vmbus_exists(void) @@ -849,30 +846,98 @@ void vmbus_device_unregister(struct hv_device *device_obj) /* - * VMBUS is an acpi enumerated device. Get the the information we - * need from DSDT. + * VMBUS is an acpi enumerated device. Get the + * information we need from DSDT. */ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) { + resource_size_t start = 0; + resource_size_t end = 0; + struct resource *new_res; + struct resource **old_res = &hyperv_mmio; + switch (res->type) { case ACPI_RESOURCE_TYPE_IRQ: irq = res->data.irq.interrupts[0]; + return AE_OK; + + /* +* "Address" descriptors are for bus windows. Ignore +* "memory" descriptors, which are for registers on +* devices. +*/ + case ACPI_RESOURCE_TYPE_ADDRESS32: + start = res->data.address32.minimum; + end = res->data.address32.maximum; break; case ACPI_RESOURCE_TYPE_ADDRESS64: - hyperv_mmio.start = res->data.address64.minimum; - hyperv_mmio.end = res->data.address64.maximum; + start = res->data.address64.minimum; + end = res->data.address64.maximum; break; + + default: + /* Unused resource type */ + return AE_OK; } + /* +* Ignore ranges that are below 1MB, as they're not +* necessary or useful here. +*/ + if (end < 0x10) + return AE_OK; + + new_res = kzalloc(sizeof(*new_res), GFP_ATOMIC); + if (!new_res) + return AE_NO_MEMORY; + + new_res->name = "hyperv mmio"; + new_res->flags = IORESOURCE_MEM; + new_res->start = start; + new_res->end = end; + + do { + if (!*old_res) { + *old_res = new_res; + break; + } + + if ((*old_res)->start > new_res->end) { + new_res->sibling = *old_res; + *old_res = new_res; + break; + } + + old_res = &(*old_res)->sibling; + + } while (1); + return AE_OK; } +static int vmbus_acpi_remove(struct acpi_device *device) +{ + struct resource *cur_res; + struct resource *next_res; + + if (hyperv_mmio) { + release_resource(hyperv_mmio); + for (cur_res = hyperv_mmio; cur_res; cur_res = next_res) { + next_res = cur_res->sibling; + kfree(cur_res); + } + } + + return 0; +} + static int vmbus_acpi_add(struct acpi_device *device) { acpi_status result; int ret_val = -ENODEV; + struct acpi_device *ancestor; hv_acpi_dev = device; @@ -882,23 +947,26 @@ static int vmbus_acpi_add(struct acpi_device *device) if (ACPI_FAILURE(result)) goto acpi_walk_err; /* -* The parent of the vmbus acpi device (Gen2 firmware) is the VMOD that -* has the mmio ranges. Get that. +* Some ancestor of the vmbus acpi device (Gen1 or Gen2 +* firmware) is the VMOD that has the mmio ranges. Get that. */ - if (device->parent) { - result = acpi_walk_resources(device->parent->handle, + for (ancestor = device->parent; ancestor; ancestor = ancestor->parent) { + result = acpi_walk_resources(ancestor->handle,
RE: [PATCH v2 1/1] drivers:hv:vmbus drivers:hv:vmbus Allow for more than one MMIO range for children
> -Original Message- > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > Sent: Friday, February 6, 2015 7:04 AM > To: Jake Oshins > Cc: gre...@linuxfoundation.org; KY Srinivasan; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com > Subject: Re: [PATCH v2 1/1] drivers:hv:vmbus drivers:hv:vmbus Allow for more > than one MMIO range for children > > Jake Oshins writes: > > > This set of changes finds the _CRS object in the ACPI namespace > > that contains memory address space descriptors, intended to convey > > to VMBus which ranges of memory-mapped I/O space are available for > > child devices, and then builds a resource list that contains all > > those ranges. Without this change, only some of the memory-mapped > > I/O space will be available for child devices, and only in some > > virtual BIOS configurations (Generation 2 VMs). > > > > This patch has been updated with feedback from Vitaly Kuznetsov. > > Cleanup is now driven by the acpi remove callback function. > > Sorry for beeing late with this message but I'm seeing issues with this > commit. I added some debug to figure out what's going on and here is > what I see: > > With Gen1 VM we end up doing request_resource for two ranges: > f800 - fffb > fe000 - fffef > > request_resource() fails (as we already have PCI device at f800 I > suppose?) but we don't check the return value. release_resource on > module unload crashes the kernel: > [ 78.314344] BUG: unable to handle kernel NULL pointer dereference at > 0030 > [ 78.315021] IP: [] release_resource+0x25/0x90 > [ 78.315021] PGD 78c67067 PUD 78c5a067 PMD 0 > [ 78.315021] Oops: [#1] SMP DEBUG_PAGEALLOC > [ 78.315021] Modules linked in: hv_vmbus(-) > ... > If I'm not mistaken, before the change we didn't do any > request_resource() for Gen1 VMs at all. > > With Gen2 VM we do request_resource for fe000 - f range > only, that means this commit doesn't change anything. > > Can you please take a look? I'd like to help but I don't completely > understand the essense of the change wrt Gen1 VMs with PCI devices. > I'll take a look immediately. Thanks, Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v2 1/1] drivers:hv:vmbus drivers:hv:vmbus Allow for more than one MMIO range for children
> > -Original Message- > > From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com] > > Sent: Friday, February 6, 2015 7:04 AM > > To: Jake Oshins > > Cc: gre...@linuxfoundation.org; KY Srinivasan; linux-ker...@vger.kernel.org; > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com > > Subject: Re: [PATCH v2 1/1] drivers:hv:vmbus drivers:hv:vmbus Allow for more > > than one MMIO range for children > > > > Sorry for beeing late with this message but I'm seeing issues with this > > commit. I added some debug to figure out what's going on and here is > > what I see: > > > > With Gen1 VM we end up doing request_resource for two ranges: > > f800 - fffb > > fe000 - fffef > > > > request_resource() fails (as we already have PCI device at f800 I > > suppose?) but we don't check the return value. release_resource on > > module unload crashes the kernel: > > [ 78.314344] BUG: unable to handle kernel NULL pointer dereference at > > 0030 > > [ 78.315021] IP: [] release_resource+0x25/0x90 > > [ 78.315021] PGD 78c67067 PUD 78c5a067 PMD 0 > > [ 78.315021] Oops: [#1] SMP DEBUG_PAGEALLOC > > [ 78.315021] Modules linked in: hv_vmbus(-) > > ... > > If I'm not mistaken, before the change we didn't do any > > request_resource() for Gen1 VMs at all. > > > > With Gen2 VM we do request_resource for fe000 - f range > > only, that means this commit doesn't change anything. > > > > Can you please take a look? I'd like to help but I don't completely > > understand the essense of the change wrt Gen1 VMs with PCI devices. > > > > I'll take a look immediately. > > Thanks, > Jake I looked at this and I realize that I need to fix this problem structurally. The intent is that VMBus-enumerated paravirtual "devices" suballocate from everything granted to the ACPI node above VMBus, in its _CRS object. But since PCI devices also have to suballocate from those ranges, I need a different approach. I'll work up a fix for this and send it around for review. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] drivers:hv Convert VMBus and its descendants to PnP
This patch adds paravirtual "devices" discovered by hv_vmbus to the pnp layer, and adds any memory-mapped I/O space claims expressed by those paravirtual devices to the "options" for that device in pnp. This allows the pnp layer to choose the memory-mapped I/O space that those paravirtual devices use. Signed-off-by: Jake Oshins --- drivers/hid/hid-hyperv.c | 6 +-- drivers/hv/channel_mgmt.c | 5 ++- drivers/hv/hyperv_vmbus.h | 1 + drivers/hv/vmbus_drv.c| 69 +++ drivers/input/serio/hyperv-keyboard.c | 24 ++-- drivers/net/hyperv/netvsc.c | 5 ++- drivers/net/hyperv/netvsc_drv.c | 4 +- drivers/net/hyperv/rndis_filter.c | 4 +- drivers/scsi/storvsc_drv.c| 2 +- drivers/video/fbdev/hyperv_fb.c | 2 +- include/linux/hyperv.h| 15 ++-- 11 files changed, 93 insertions(+), 44 deletions(-) diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c index 6039f07..0cf9105 100644 --- a/drivers/hid/hid-hyperv.c +++ b/drivers/hid/hid-hyperv.c @@ -309,7 +309,7 @@ static void mousevsc_on_receive(struct hv_device *device, hid_input_report(input_dev->hid_device, HID_INPUT_REPORT, input_dev->input_buf, len, 1); - pm_wakeup_event(&input_dev->device->device, 0); + pm_wakeup_event(&input_dev->device->pnp_dev->dev, 0); break; default: @@ -552,7 +552,7 @@ static int mousevsc_probe(struct hv_device *device, goto probe_err2; } - device_init_wakeup(&device->device, true); + device_init_wakeup(&device->pnp_dev->dev, true); input_dev->connected = true; input_dev->init_complete = true; @@ -576,7 +576,7 @@ static int mousevsc_remove(struct hv_device *dev) { struct mousevsc_dev *input_dev = hv_get_drvdata(dev); - device_init_wakeup(&dev->device, false); + device_init_wakeup(&dev->pnp_dev->dev, false); vmbus_close(dev->channel); hid_hw_stop(input_dev->hid_device); hid_destroy_device(input_dev->hid_device); diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 3736f71..fcb1be8 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -218,8 +218,8 @@ static void vmbus_process_rescind_offer(struct work_struct *work) struct vmbus_channel_relid_released msg; struct device *dev; - if (channel->device_obj) { - dev = get_device(&channel->device_obj->device); + if (channel->device_obj && channel->device_obj->pnp_dev) { + dev = get_device(&channel->device_obj->pnp_dev->dev); if (dev) { vmbus_device_unregister(channel->device_obj); put_device(dev); @@ -359,6 +359,7 @@ static void vmbus_process_offer(struct work_struct *work) newchannel->device_obj = vmbus_device_create( &newchannel->offermsg.offer.if_type, &newchannel->offermsg.offer.if_instance, + newchannel->offermsg.offer.mmio_megabytes, newchannel); if (!newchannel->device_obj) goto err_free_chan; diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 44b1c94..73b9bc0 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -676,6 +676,7 @@ extern struct vmbus_connection vmbus_connection; struct hv_device *vmbus_device_create(const uuid_le *type, const uuid_le *instance, + u16 mmio_mb, struct vmbus_channel *channel); int vmbus_device_register(struct hv_device *child_device_obj); diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index f518b8d7..5d85ef3 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -549,10 +549,13 @@ static void vmbus_device_release(struct device *device) { struct hv_device *hv_dev = device_to_hv_device(device); - kfree(hv_dev); + if (hv_dev->pnp_dev) + free_pnp_descendant(hv_dev->pnp_dev); + kfree(hv_dev); } + /* The one and only one */ static struct bus_type hv_bus = { .name = "vmbus", @@ -814,6 +817,7 @@ EXPORT_SYMBOL_GPL(vmbus_driver_unregister); */ struct hv_device *vmbus_device_create(const uuid_le *type, const uuid_le *instance, + u16 mmio_mb, struct vmbus_channel *channel) { struct hv_device *child_device_obj; @@ -825,6 +829,7 @@ struct hv_device *vmbus_device_create(const uuid_le *type,
[PATCH 1/3] drivers:pnp Add support for descendants claiming memory address space
This patch adds some wrapper functions in the pnp layer. The intent is to allow memory address space claims by devices which are descendants (a child or grandchild of) a device which is already part of the pnp layer. This allows a device to make a resource claim that doesn't conflict with its "aunts" and "uncles." This is useful in a Hyper-V VM because some paravirtual "devices" need memory-mapped I/O space, and their aunts and uncles can be PCI devices. Furthermore, the hypervisor expresses the possible memory address combinations for the devices in the VM through the ACPI namespace. The paravirtual devices need to suballocate from the ACPI nodes, and they need to avoid conflicting with choices that the Linux PCI code makes about the PCI devices in the VM. It might seem like this should be done in the platform layer rather than the pnp layer, but the platform layer assumes that the configuration of the devices in the machine are static, or at least expressed by firmware in a static fashion. The nature of a Hyper-V VM is that new devices can be added while the machine is running, and the potential configurations for them are expressed as part of the paravirtual communications channel. This much more naturally aligns with the pnp layer. Signed-off-by: Jake Oshins --- drivers/pnp/Makefile | 2 +- drivers/pnp/base.h | 2 + drivers/pnp/core.c | 1 + drivers/pnp/descendant.c | 117 +++ include/linux/pnp.h | 23 ++ 5 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 drivers/pnp/descendant.c diff --git a/drivers/pnp/Makefile b/drivers/pnp/Makefile index bfba893..de315f9 100644 --- a/drivers/pnp/Makefile +++ b/drivers/pnp/Makefile @@ -4,7 +4,7 @@ obj-y := pnp.o -pnp-y := core.o card.o driver.o resource.o manager.o support.o interface.o quirks.o +pnp-y := core.o card.o driver.o resource.o manager.o support.o interface.o quirks.o descendant.o obj-$(CONFIG_PNPACPI) += pnpacpi/ obj-$(CONFIG_PNPBIOS) += pnpbios/ diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h index c8873b0..0b86437 100644 --- a/drivers/pnp/base.h +++ b/drivers/pnp/base.h @@ -175,6 +175,8 @@ struct pnp_resource *pnp_add_bus_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t end); +int __init pnpdescendant_init(void); + extern int pnp_debug; #if defined(CONFIG_PNP_DEBUG_MESSAGES) diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c index cb6ce42..fc32912 100644 --- a/drivers/pnp/core.c +++ b/drivers/pnp/core.c @@ -212,6 +212,7 @@ void __pnp_remove_device(struct pnp_dev *dev) static int __init pnp_init(void) { + pnpdescendant_init(); return bus_register(&pnp_bus_type); } diff --git a/drivers/pnp/descendant.c b/drivers/pnp/descendant.c new file mode 100644 index 000..c25e40b --- /dev/null +++ b/drivers/pnp/descendant.c @@ -0,0 +1,117 @@ +/* + * descendant.c - Resource management for devices which are descendants + * (children or grandchildren of) devices which directly allocate resources, + * i.e. devices enumerated by ACPI, PCI, PCMCIA, ISAPNP or PNPBIOS + * + * Copyright (C) 2015 Microsoft + * Jake Oshins + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include "base.h" + +int descendant_get_resources(struct pnp_dev *dev) +{ + return 0; +} + +int descendant_set_resources(struct pnp_dev *dev) +{ + return 0; +} + +int descendant_disable_resources(struct pnp_dev *dev) +{ + return 0; +} + +struct pnp_protocol descendant_protocol = { + .name= "Plug and Play descendants", + .get = descendant_get_resources, + .set = descendant_set_resources, + .disable = descendant_disable_resources, +}; + +/* + * This adds an option to the descendant device for memory address space. +*/ +int pnp_descendant_memory_option(struct pnp_dev *dev, resource_size_t min, +resource_size_t max, resource_size_t alignment, +resource_size_t size, unsigned char flags) +{ + return pnp_register_mem_resource(dev, 0, min, max, alignment, size, +flags); +} +EXPORT_SYMBOL(pnp_descendant_memory_option); + +/* + * This creates an entry in the plug-and-play layer for this device, which +
[PATCH 3/3] drivers:hv Remove old MMIO management code
This patch removes the code that is no longer necessary after the first two patches in this series have been applied. It exposed a static range of memory-mapped I/O space gleaned from the ACPI namespace, in a way that worked for a single paravirtual device, the video frame buffer. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 25 - drivers/video/fbdev/hyperv_fb.c | 27 ++- include/linux/hyperv.h | 2 -- 3 files changed, 14 insertions(+), 40 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 5d85ef3..2722e63 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -44,12 +44,6 @@ static struct tasklet_struct msg_dpc; static struct completion probe_event; static int irq; -struct resource hyperv_mmio = { - .name = "hyperv mmio", - .flags = IORESOURCE_MEM, -}; -EXPORT_SYMBOL_GPL(hyperv_mmio); - static int vmbus_exists(void) { if (hv_acpi_dev == NULL) @@ -555,7 +549,6 @@ static void vmbus_device_release(struct device *device) kfree(hv_dev); } - /* The one and only one */ static struct bus_type hv_bus = { .name = "vmbus", @@ -931,11 +924,6 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) case ACPI_RESOURCE_TYPE_IRQ: irq = res->data.irq.interrupts[0]; break; - - case ACPI_RESOURCE_TYPE_ADDRESS64: - hyperv_mmio.start = res->data.address64.address.minimum; - hyperv_mmio.end = res->data.address64.address.maximum; - break; } return AE_OK; @@ -953,20 +941,7 @@ static int vmbus_acpi_add(struct acpi_device *device) if (ACPI_FAILURE(result)) goto acpi_walk_err; - /* -* The parent of the vmbus acpi device (Gen2 firmware) is the VMOD that -* has the mmio ranges. Get that. -*/ - if (device->parent) { - result = acpi_walk_resources(device->parent->handle, - METHOD_NAME__CRS, - vmbus_walk_resources, NULL); - if (ACPI_FAILURE(result)) - goto acpi_walk_err; - if (hyperv_mmio.start && hyperv_mmio.end) - request_resource(&iomem_resource, &hyperv_mmio); - } ret_val = 0; acpi_walk_err: diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c index 69ea59b..161157e 100644 --- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c @@ -675,26 +675,22 @@ static void hvfb_get_option(struct fb_info *info) /* Get framebuffer memory from Hyper-V video pci space */ -static int hvfb_getmem(struct fb_info *info) +static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) { struct hvfb_par *par = info->par; struct pci_dev *pdev = NULL; void __iomem *fb_virt; + struct resource *res; int gen2vm = efi_enabled(EFI_BOOT); int ret; - par->mem.name = KBUILD_MODNAME; - par->mem.flags = IORESOURCE_MEM | IORESOURCE_BUSY; if (gen2vm) { - ret = allocate_resource(&hyperv_mmio, &par->mem, - screen_fb_size, - 0, -1, - screen_fb_size, - NULL, NULL); - if (ret != 0) { - pr_err("Unable to allocate framebuffer memory\n"); + res = pnp_get_resource(hdev->pnp_dev, IORESOURCE_MEM, 0); + if (!res) { + pr_err("Unable to fetch FB claim\n"); return -ENODEV; } + par->mem = *res; } else { pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT, PCI_DEVICE_ID_HYPERV_VIDEO, NULL); @@ -707,6 +703,8 @@ static int hvfb_getmem(struct fb_info *info) pci_resource_len(pdev, 0) < screen_fb_size) goto err1; + par->mem.name = KBUILD_MODNAME; + par->mem.flags = IORESOURCE_MEM | IORESOURCE_BUSY; par->mem.end = pci_resource_end(pdev, 0); par->mem.start = par->mem.end - screen_fb_size + 1; ret = request_resource(&pdev->resource[0], &par->mem); @@ -747,7 +745,8 @@ static int hvfb_getmem(struct fb_info *info) err3: iounmap(fb_virt); err2: - release_resource(&par->mem); + if (!gen2vm) + release_resource(&par->mem); err1: if (!gen2vm) pci_dev_put(pdev); @@ -759,9 +758,11 @@ err1: static void hvfb_putmem(struct fb_info *info) { struct hvfb_par *
RE: [PATCH 2/3] drivers:hv Convert VMBus and its descendants to PnP
> -Original Message- > From: Dan Carpenter [mailto:dan.carpen...@oracle.com] > Sent: Wednesday, February 18, 2015 2:24 AM > To: Jake Oshins > Cc: rafael.j.wyso...@intel.com; gre...@linuxfoundation.org; KY Srinivasan; > linux-ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@redhat.com > Subject: Re: [PATCH 2/3] drivers:hv Convert VMBus and its descendants to PnP > > On Tue, Feb 17, 2015 at 11:41:50AM -0800, Jake Oshins wrote: > > > > + ret = pnp_add_descendant(child_device_obj->pnp_dev); > > if (ret) > > + goto register_exit; > > + > > + added = TRUE; > > + > > + ret = pnp_activate_dev(child_device_obj->pnp_dev); > > + > > +register_exit: > > + > > + if (ret) { > > + if (added) > > + pnp_remove_descendant(child_device_obj->pnp_dev); > > + free_pnp_descendant(child_device_obj->pnp_dev); > > pr_err("Unable to register child device\n"); > > - else > > + } else > > pr_debug("child device %s registered\n", > > - dev_name(&child_device_obj->device)); > > +dev_name(&child_device_obj->pnp_dev->dev)); > > > > return ret; > > } > > This error handling is horribly ugly and has a bug. Please, read my > essay on how to do error handling properly. > > https://plus.google.com/106378716002406849458/posts/dnanfhQ4mHQ > > 1) Don't do "One Err" style error handling. > 2) Don't twist the success path and the error paths together. > 3) Don't use the "added" variable. > 4) Don't initialize "ret" at the start of the function. > 5) Don't forget to undo pnp_descendant_memory_option() > > It should look something like: > > return 0; > > err_remove_desc: > pnp_remove_descendant(child_device_obj->pnp_dev); > err_remove_option: > if (bytes) > remove_whatever(); > err_free_desc: > free_pnp_descendant(child_device_obj->pnp_dev); > > return ret; > > regards, > dan carpenter Will do. Thanks for the review. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/3] drivers:pnp Add support for descendants claiming memory address space
This patch adds the concept of "descendant" device to the pnp layer, one which can claim memory mapped I/O space from the same regions of address space that its "aunts" and "uncles" are claiming from. This is more or less analogous to a device on an ISA bus that needs address space, but needs to avoid claiming any regions that are in use by PCI devices which are peers of the PCI to ISA bridge. This is done in support of changes in the Hyper-V-related drivers, but nothing in this patch is specific to Hyper-V, except in that it doesn't seem to be needed for anything else. Signed-off-by: Jake Oshins --- drivers/pnp/Makefile | 2 +- drivers/pnp/base.h | 2 + drivers/pnp/core.c | 1 + drivers/pnp/descendant.c | 117 +++ include/linux/pnp.h | 23 ++ 5 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 drivers/pnp/descendant.c diff --git a/drivers/pnp/Makefile b/drivers/pnp/Makefile index bfba893..de315f9 100644 --- a/drivers/pnp/Makefile +++ b/drivers/pnp/Makefile @@ -4,7 +4,7 @@ obj-y := pnp.o -pnp-y := core.o card.o driver.o resource.o manager.o support.o interface.o quirks.o +pnp-y := core.o card.o driver.o resource.o manager.o support.o interface.o quirks.o descendant.o obj-$(CONFIG_PNPACPI) += pnpacpi/ obj-$(CONFIG_PNPBIOS) += pnpbios/ diff --git a/drivers/pnp/base.h b/drivers/pnp/base.h index c8873b0..0b86437 100644 --- a/drivers/pnp/base.h +++ b/drivers/pnp/base.h @@ -175,6 +175,8 @@ struct pnp_resource *pnp_add_bus_resource(struct pnp_dev *dev, resource_size_t start, resource_size_t end); +int __init pnpdescendant_init(void); + extern int pnp_debug; #if defined(CONFIG_PNP_DEBUG_MESSAGES) diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c index cb6ce42..fc32912 100644 --- a/drivers/pnp/core.c +++ b/drivers/pnp/core.c @@ -212,6 +212,7 @@ void __pnp_remove_device(struct pnp_dev *dev) static int __init pnp_init(void) { + pnpdescendant_init(); return bus_register(&pnp_bus_type); } diff --git a/drivers/pnp/descendant.c b/drivers/pnp/descendant.c new file mode 100644 index 000..c25e40b --- /dev/null +++ b/drivers/pnp/descendant.c @@ -0,0 +1,117 @@ +/* + * descendant.c - Resource management for devices which are descendants + * (children or grandchildren of) devices which directly allocate resources, + * i.e. devices enumerated by ACPI, PCI, PCMCIA, ISAPNP or PNPBIOS + * + * Copyright (C) 2015 Microsoft + * Jake Oshins + * + * This program is free software; you can redistribute it and/or modify it + * under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for + * more details. + * + */ + +#include +#include +#include +#include +#include +#include +#include +#include "base.h" + +int descendant_get_resources(struct pnp_dev *dev) +{ + return 0; +} + +int descendant_set_resources(struct pnp_dev *dev) +{ + return 0; +} + +int descendant_disable_resources(struct pnp_dev *dev) +{ + return 0; +} + +struct pnp_protocol descendant_protocol = { + .name= "Plug and Play descendants", + .get = descendant_get_resources, + .set = descendant_set_resources, + .disable = descendant_disable_resources, +}; + +/* + * This adds an option to the descendant device for memory address space. +*/ +int pnp_descendant_memory_option(struct pnp_dev *dev, resource_size_t min, +resource_size_t max, resource_size_t alignment, +resource_size_t size, unsigned char flags) +{ + return pnp_register_mem_resource(dev, 0, min, max, alignment, size, +flags); +} +EXPORT_SYMBOL(pnp_descendant_memory_option); + +/* + * This creates an entry in the plug-and-play layer for this device, which + * is a descendant of a device already in the plug-and-play layer. +*/ +int pnp_add_descendant(struct pnp_dev *dev) +{ + int error; + + error = pnp_add_device(dev); + if (error) { + put_device(&dev->dev); + return error; + } + + return 0; +} +EXPORT_SYMBOL(pnp_add_descendant); + +void pnp_remove_descendant(struct pnp_dev *dev) +{ + __pnp_remove_device(dev); +} +EXPORT_SYMBOL(pnp_remove_descendant); + +int __init pnpdescendant_init(void) +{ + return pnp_register_protocol(&descendant_protocol); +} + +struct pnp_dev *alloc_pnp_descendant(const char *pnpid) +{ + struct pnp_dev *dev; + +
[PATCH v2 0/3] Convert Hyper-V code to use the pnp layer
This set of patches changes the way that drivers in a Hyper-V VM find the memory-mapped I/O space that they need. The Hyper-V BIOS and UEFI implementations expose a couple of large regions of MMIO space to the guests using the ACPI namespace, with the expectation that the guest OS will subdivide those regions for each of the paravirtual devices that require some. The firmware itself does not pick the specific regions for each device. The old Hyper-V code that this patch set replaces found that region by directly examining the ACPI namespace and exporting one of the ranges for use by a single frame buffer driver. This worked, so long as there was only one client driver instance that wanted address space. In order to allow multiple clients to each pick a region of memory-mapped I/O space, the first patch adds a few wrapper functions in the kernel's pnp layer which allow address space requests ("options") for drivers which are descendants of the busses that the pnp layer already understands. This ensures that descendants can make requests for MMIO space which don't conflict with "aunts" and "uncles" in the device tree. (Hyper-V VMs can have PCI devices which need to claim from the space address space.) The second patch converts hv_vmbus and all the related Hyper-V drivers to use the pnp layer. The third patch removes the older code which examined the ACPI namespace directly. Jake Oshins (3): drivers:pnp Add support for descendants claiming memory address space drivers:hv Convert VMBus and its descendants to PnP drivers:hv Remove old MMIO management code drivers/hid/hid-hyperv.c | 6 +- drivers/hv/channel_mgmt.c | 5 +- drivers/hv/hyperv_vmbus.h | 1 + drivers/hv/vmbus_drv.c| 145 +++--- drivers/input/serio/hyperv-keyboard.c | 24 +++--- drivers/net/hyperv/netvsc.c | 5 +- drivers/net/hyperv/netvsc_drv.c | 4 +- drivers/net/hyperv/rndis_filter.c | 4 +- drivers/pnp/Makefile | 2 +- drivers/pnp/base.h| 2 + drivers/pnp/core.c| 1 + drivers/pnp/descendant.c | 117 +++ drivers/scsi/storvsc_drv.c| 2 +- drivers/video/fbdev/hyperv_fb.c | 29 +++ include/linux/hyperv.h| 17 ++-- include/linux/pnp.h | 23 ++ 16 files changed, 261 insertions(+), 126 deletions(-) create mode 100644 drivers/pnp/descendant.c -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/3] drivers:hv Remove old MMIO management code
This patch removes the now-redundant code which examined the ACPI namespace directly for memory-mapped I/O regions for its children. Signed-off-by: Jake Oshins --- drivers/hv/vmbus_drv.c | 28 ++-- include/linux/hyperv.h | 2 -- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 7783f4c..193a362 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -44,12 +44,6 @@ static struct tasklet_struct msg_dpc; static struct completion probe_event; static int irq; -struct resource hyperv_mmio = { - .name = "hyperv mmio", - .flags = IORESOURCE_MEM, -}; -EXPORT_SYMBOL_GPL(hyperv_mmio); - static int vmbus_exists(void) { if (hv_acpi_dev == NULL) @@ -899,11 +893,6 @@ static acpi_status vmbus_walk_resources(struct acpi_resource *res, void *ctx) case ACPI_RESOURCE_TYPE_IRQ: irq = res->data.irq.interrupts[0]; break; - - case ACPI_RESOURCE_TYPE_ADDRESS64: - hyperv_mmio.start = res->data.address64.address.minimum; - hyperv_mmio.end = res->data.address64.address.maximum; - break; } return AE_OK; @@ -917,24 +906,11 @@ static int vmbus_acpi_add(struct acpi_device *device) hv_acpi_dev = device; result = acpi_walk_resources(device->handle, METHOD_NAME__CRS, - vmbus_walk_resources, NULL); +vmbus_walk_resources, NULL); if (ACPI_FAILURE(result)) goto acpi_walk_err; - /* -* The parent of the vmbus acpi device (Gen2 firmware) is the VMOD that -* has the mmio ranges. Get that. -*/ - if (device->parent) { - result = acpi_walk_resources(device->parent->handle, - METHOD_NAME__CRS, - vmbus_walk_resources, NULL); - - if (ACPI_FAILURE(result)) - goto acpi_walk_err; - if (hyperv_mmio.start && hyperv_mmio.end) - request_resource(&iomem_resource, &hyperv_mmio); - } + ret_val = 0; acpi_walk_err: diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index 796cc32..993ea5f 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -1221,8 +1221,6 @@ int hv_vss_init(struct hv_util_service *); void hv_vss_deinit(void); void hv_vss_onchannelcallback(void *); -extern struct resource hyperv_mmio; - /* * Negotiated version with the Host. */ -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/3] drivers:hv Convert VMBus and its descendants to PnP
This patch makes hv_vmbus and the devices that it discovers use the pnp layer, so that address space claims made by these devices will hook into the regions of address space exposed by ACPI, which already hooks into the pnp layer. Previous feedback from Dan Carpenter has been incorporated. (Thank you.) Signed-off-by: Jake Oshins --- drivers/hid/hid-hyperv.c | 6 +- drivers/hv/channel_mgmt.c | 5 +- drivers/hv/hyperv_vmbus.h | 1 + drivers/hv/vmbus_drv.c| 117 ++ drivers/input/serio/hyperv-keyboard.c | 24 +++ drivers/net/hyperv/netvsc.c | 5 +- drivers/net/hyperv/netvsc_drv.c | 4 +- drivers/net/hyperv/rndis_filter.c | 4 +- drivers/scsi/storvsc_drv.c| 2 +- drivers/video/fbdev/hyperv_fb.c | 29 + include/linux/hyperv.h| 15 +++-- 11 files changed, 115 insertions(+), 97 deletions(-) diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c index 6039f07..0cf9105 100644 --- a/drivers/hid/hid-hyperv.c +++ b/drivers/hid/hid-hyperv.c @@ -309,7 +309,7 @@ static void mousevsc_on_receive(struct hv_device *device, hid_input_report(input_dev->hid_device, HID_INPUT_REPORT, input_dev->input_buf, len, 1); - pm_wakeup_event(&input_dev->device->device, 0); + pm_wakeup_event(&input_dev->device->pnp_dev->dev, 0); break; default: @@ -552,7 +552,7 @@ static int mousevsc_probe(struct hv_device *device, goto probe_err2; } - device_init_wakeup(&device->device, true); + device_init_wakeup(&device->pnp_dev->dev, true); input_dev->connected = true; input_dev->init_complete = true; @@ -576,7 +576,7 @@ static int mousevsc_remove(struct hv_device *dev) { struct mousevsc_dev *input_dev = hv_get_drvdata(dev); - device_init_wakeup(&dev->device, false); + device_init_wakeup(&dev->pnp_dev->dev, false); vmbus_close(dev->channel); hid_hw_stop(input_dev->hid_device); hid_destroy_device(input_dev->hid_device); diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index 3736f71..fcb1be8 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -218,8 +218,8 @@ static void vmbus_process_rescind_offer(struct work_struct *work) struct vmbus_channel_relid_released msg; struct device *dev; - if (channel->device_obj) { - dev = get_device(&channel->device_obj->device); + if (channel->device_obj && channel->device_obj->pnp_dev) { + dev = get_device(&channel->device_obj->pnp_dev->dev); if (dev) { vmbus_device_unregister(channel->device_obj); put_device(dev); @@ -359,6 +359,7 @@ static void vmbus_process_offer(struct work_struct *work) newchannel->device_obj = vmbus_device_create( &newchannel->offermsg.offer.if_type, &newchannel->offermsg.offer.if_instance, + newchannel->offermsg.offer.mmio_megabytes, newchannel); if (!newchannel->device_obj) goto err_free_chan; diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h index 44b1c94..73b9bc0 100644 --- a/drivers/hv/hyperv_vmbus.h +++ b/drivers/hv/hyperv_vmbus.h @@ -676,6 +676,7 @@ extern struct vmbus_connection vmbus_connection; struct hv_device *vmbus_device_create(const uuid_le *type, const uuid_le *instance, + u16 mmio_mb, struct vmbus_channel *channel); int vmbus_device_register(struct hv_device *child_device_obj); diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index f518b8d7..7783f4c 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -487,6 +487,13 @@ static int vmbus_probe(struct device *child_device) struct hv_device *dev = device_to_hv_device(child_device); const struct hv_vmbus_device_id *dev_id; + ret = pnp_activate_dev(dev->pnp_dev); + if (ret) { + pr_err("Unable to activate child device %s\n", + dev_name(&dev->pnp_dev->dev)); + return ret; + } + dev_id = hv_vmbus_get_id(drv->id_table, dev->dev_type.b); if (drv->probe) { ret = drv->probe(dev, dev_id); @@ -510,6 +517,8 @@ static int vmbus_remove(struct device *child_device) struct hv_driver *drv = drv_to_hv_drv(child_device->driver); struct hv_device *dev = device_to_hv_device(child_device); + pnp_disable_dev(dev->pnp_dev)
RE: [PATCH 1/3] drivers:pnp Add support for descendants claiming memory address space
> -Original Message- > From: Rafael J. Wysocki [mailto:rafael.j.wyso...@intel.com] > Sent: Thursday, March 5, 2015 3:04 PM > To: Jake Oshins > Cc: gre...@linuxfoundation.org; KY Srinivasan; linux- > ker...@vger.kernel.org; de...@linuxdriverproject.org; o...@aepfle.de; > a...@canonical.com; vkuzn...@redhat.com; Rafael J. Wysocki; Linux ACPI > Subject: Re: [PATCH 1/3] drivers:pnp Add support for descendants claiming > memory address space > > On 2/17/2015 8:41 PM, Jake Oshins wrote: > > This patch adds some wrapper functions in the pnp layer. The intent is > > to allow memory address space claims by devices which are descendants > > (a child or grandchild of) a device which is already part of the pnp > > layer. This allows a device to make a resource claim that doesn't > > conflict with its "aunts" and "uncles." > > How is this going to happen? First, thanks for the review. As for your question, I'm not sure whether you're asking how the code that I supplied makes it happen or how we might happen to be in a situation where you want to make a resource claim that doesn't conflict with aunts and uncles. I'll address the second interpretation first. Imagine you have a PC from the mid '90s, or any time period when PCI coexisted with other bus technologies like ISA and EISA. You have a region of memory address space that's reserved for I/O devices. PCI devices sit below that. So do bridges to other bus technologies. When picking a memory region for one of the PCI devices, you need to ensure that it doesn't overlap both other PCI devices and devices which are beneath the EISA/ISA/other bridge. The PCI devices are the aunts and uncles. The EISA/ISA devices are nephews and nieces. The bridge to the EISA/ISA bus claims nothing and just passes through cycles that aren't claimed by PCI devices. A Hyper-V VM is much like that mid '90s PC. "Generation 1 VMs" in Hyper-V are like it because they are, in fact, an emulation of a specific PC from 1997. "Generation 2 VMs" in Hyper-V are like it because they have a single region reported by the virtual UEFI firmware to be used by everything below that, with nothing else described by the firmware, except a "bridge" to a virtual bus called VMBus, which itself has no address space description, much like the EISA/ISA bus had no address space description. Devices can be added or removed from the VM while it is running, and they have no description in ACPI. As for how the code I supplied makes this happen, it adds a more generic wrapper to the pnp layer, augmenting the four wrappers which already exist; ISAPNP/PNPBIOS, PCI, PCMCIA and ACPI. Each of these four wrappers has code specific to the bus type. I just added a small wrapper that doesn't have that. > > > This is useful in a Hyper-V VM because some paravirtual "devices" need > > memory-mapped I/O space, and their aunts and uncles can be PCI devices. > > Furthermore, the hypervisor expresses the possible memory address > > combinations for the devices in the VM through the ACPI namespace. > > The paravirtual devices need to suballocate from the ACPI nodes, and > > they need to avoid conflicting with choices that the Linux PCI code > > makes about the PCI devices in the VM. > > > > It might seem like this should be done in the platform layer rather > > than the pnp layer, but the platform layer assumes that the > > configuration of the devices in the machine are static, or at least > > expressed by firmware in a static fashion. > > I'm not sure if I'm following you here. > > Where exactly do we make that assumption? > > Yes, some code to support platform device hotplug may be missing, but I'd > very much prefer to add it instead of reviving the dead man walking which is > the PNP subsystem today. > I'm completely open to adding this to the platform layer instead of the pnp layer. But it seems like you'd have to accommodate a usage model that doesn't yet exist in the platform layer. (You confirmed for me yourself in another e-mail that the platform layer doesn't have a provision for asking for things like "any 100MB of memory address space under my root bus, and I won't bloat this message by pasting that in unless you'd like me to send it along.) If I were to try to use the platform layer without heavily modifying it, I'd have to write code that, from the client, driver, tried to probe for regions of free address space. Imagine an algorithm that attempted to find a free 16K by just calling allocate_resources() for every 16K region, starting from the top (or bottom) of address space until one of those claims su
RE: [PATCH] PCI: hv: Fix interrupt cleanup path
> -Original Message- > From: Cathy Avery [mailto:cav...@redhat.com] > Sent: Tuesday, July 12, 2016 8:31 AM > To: KY Srinivasan ; Haiyang Zhang > ; Jake Oshins ; > bhelg...@google.com > Cc: vkuzn...@redhat.com; de...@linuxdriverproject.org; linux- > p...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: [PATCH] PCI: hv: Fix interrupt cleanup path > > SR-IOV disabled from the host causes a memory leak. > pci-hyperv usually first receives a PCI_EJECT notification > and then proceeds to delete the hpdev list entry in > hv_eject_device_work(). Later in hv_msi_free() since the > device is no longer on the device list hpdev is NULL > and hv_msi_free returns without freeing int_desc as part of > hv_int_desc_free(). > > Signed-off-by: Cathy Avery Acked-by: Jake Oshins > --- > drivers/pci/host/pci-hyperv.c | 14 -- > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 7e9b2de..449d053 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -732,16 +732,18 @@ static void hv_msi_free(struct irq_domain *domain, > struct msi_domain_info *info, > > pdev = msi_desc_to_pci_dev(msi); > hbus = info->data; > - hpdev = get_pcichild_wslot(hbus, devfn_to_wslot(pdev->devfn)); > - if (!hpdev) > + int_desc = irq_data_get_irq_chip_data(irq_data); > + if (!int_desc) > return; > > - int_desc = irq_data_get_irq_chip_data(irq_data); > - if (int_desc) { > - irq_data->chip_data = NULL; > - hv_int_desc_free(hpdev, int_desc); > + irq_data->chip_data = NULL; > + hpdev = get_pcichild_wslot(hbus, devfn_to_wslot(pdev->devfn)); > + if (!hpdev) { > + kfree(int_desc); > + return; > } > > + hv_int_desc_free(hpdev, int_desc); > put_pcichild(hpdev, hv_pcidev_ref_by_slot); > } > > -- > 2.5.0 I've tested this. It's a good fix. Please take it. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/3] PCI: hv: use the correct buffer size in new_pcichild_device()
> -Original Message- > From: Dexuan Cui > Sent: Wednesday, November 9, 2016 11:18 PM > To: Bjorn Helgaas ; linux-...@vger.kernel.org; > de...@linuxdriverproject.org > Cc: gre...@linuxfoundation.org; KY Srinivasan ; > Haiyang Zhang ; Stephen Hemminger > ; Jake Oshins ; Hadden > Hoppert ; Vitaly Kuznetsov > ; jasow...@redhat.com; a...@canonical.com; > o...@aepfle.de; linux-ker...@vger.kernel.org > Subject: [PATCH 1/3] PCI: hv: use the correct buffer size in > new_pcichild_device() > > We don't really need such a big on-stack buffer. > vmbus_sendpacket() here only uses sizeof(struct pci_child_message). > > Signed-off-by: Dexuan Cui > CC: Jake Oshins > Cc: KY Srinivasan > CC: Haiyang Zhang > CC: Vitaly Kuznetsov > --- > drivers/pci/host/pci-hyperv.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 763ff87..93ed64a 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1271,9 +1271,9 @@ static struct hv_pci_dev > *new_pcichild_device(struct hv_pcibus_device *hbus, > struct hv_pci_dev *hpdev; > struct pci_child_message *res_req; > struct q_res_req_compl comp_pkt; > - union { > - struct pci_packet init_packet; > - u8 buffer[0x100]; > + struct { > + struct pci_packet init_packet; > + u8 buffer[sizeof(struct pci_child_message)]; > } pkt; > unsigned long flags; > int ret; > -- > 2.7.4 This change seems good to me, in that it's always a bad idea to use too much stack. But this won't fix the problem with VMAP_STACK. The buffer could still end up spanning two pages and the physical addresses of those pages would possibly be discontiguous. Do you want to just refactor this so that it uses a fixed buffer, one that will work with VMAP_STACK? Or is that coming in a future patch? -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/3] PCI: hv: use the correct buffer size in new_pcichild_device()
> -Original Message- > > > From: Jake Oshins > > > From: Dexuan Cui > > > Sent: Wednesday, November 9, 2016 11:18 PM > > > We don't really need such a big on-stack buffer. > > > vmbus_sendpacket() here only uses sizeof(struct pci_child_message). > > > > > > @@ -1271,9 +1271,9 @@ static struct hv_pci_dev > > > *new_pcichild_device(struct hv_pcibus_device *hbus, > > > struct hv_pci_dev *hpdev; > > > struct pci_child_message *res_req; > > > struct q_res_req_compl comp_pkt; > > > - union { > > > - struct pci_packet init_packet; > > > - u8 buffer[0x100]; > > > + struct { > > > + struct pci_packet init_packet; > > > + u8 buffer[sizeof(struct pci_child_message)]; > > > } pkt; > > > unsigned long flags; > > > int ret; > > > > This change seems good to me, in that it's always a bad idea to use too > much > > stack. But this won't fix the problem with VMAP_STACK. The buffer could > still > > end up spanning two pages and the physical addresses of those pages > would > > possibly be discontiguous. Do you want to just refactor this so that it > > uses > a > > fixed buffer, one that will work with VMAP_STACK? Or is that coming in a > future > > patch? > > Hi Jake, I think the VMAP_STACK issue you mentioned should be another > different > issue fixed by Long Li: https://patchwork.ozlabs.org/patch/692447/. > > The VMAP_STACK issue is only an issue when we pass the buffer's physical > address > to the hypercall. > > Here the buffer is not passed to any hypercall. We just use > vmbus_sendpacket() > to memcpy the buffer into the per-channel ringbuffer. > > Thanks, > -- Dexuan Yes, you're right. This patch looks fine to me. Sorry for confusing the two issues. -- Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 2/3] PCI: hv: fix hv_pci_remove() for hot-remove
> -Original Message- > From: Dexuan Cui > Sent: Wednesday, November 9, 2016 11:19 PM > To: Bjorn Helgaas ; linux-...@vger.kernel.org; > de...@linuxdriverproject.org > Cc: gre...@linuxfoundation.org; KY Srinivasan ; > Haiyang Zhang ; Stephen Hemminger > ; Jake Oshins ; Hadden > Hoppert ; Vitaly Kuznetsov > ; jasow...@redhat.com; a...@canonical.com; > o...@aepfle.de; linux-ker...@vger.kernel.org > Subject: [PATCH 2/3] PCI: hv: fix hv_pci_remove() for hot-remove > > 1. We don't really need such a big on-stack buffer when sending > the teardown_packet: vmbus_sendpacket() here only uses > sizeof(struct pci_message). > > 2. In the hot-remove case (PCI_EJECT), after we send > PCI_EJECTION_COMPLETE > to the host, the host will send a RESCIND_CHANNEL message to us and the > host won't access the per-channel ringbuffer any longer, so we needn't > send PCI_RESOURCES_RELEASED/PCI_BUS_D0EXIT to the host, and we > shouldn't > expect the host's completion message of PCI_BUS_D0EXIT, which will never > come. > > 3. We should send PCI_BUS_D0EXIT after hv_send_resources_released(). > > Signed-off-by: Dexuan Cui > CC: Jake Oshins > Cc: KY Srinivasan > CC: Haiyang Zhang > CC: Vitaly Kuznetsov > > --- > > I made the patch based on discussions with Jake Oshins and others. > > drivers/pci/host/pci-hyperv.c | 53 +++ > > 1 file changed, 33 insertions(+), 20 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 93ed64a..7590ad0 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -2266,24 +2266,32 @@ static int hv_pci_probe(struct hv_device *hdev, > return ret; > } > > -/** > - * hv_pci_remove() - Remove routine for this VMBus channel > - * @hdev:VMBus's tracking struct for this root PCI bus > - * > - * Return: 0 on success, -errno on failure > - */ > -static int hv_pci_remove(struct hv_device *hdev) > +static void hv_pci_bus_exit(struct hv_device *hdev) > { > - int ret; > - struct hv_pcibus_device *hbus; > - union { > + struct hv_pcibus_device *hbus = hv_get_drvdata(hdev); > + struct { > struct pci_packet teardown_packet; > - u8 buffer[0x100]; > + u8 buffer[sizeof(struct pci_message)]; > } pkt; > struct pci_bus_relations relations; > struct hv_pci_compl comp_pkt; > + int ret; > > - hbus = hv_get_drvdata(hdev); > + /* > + * After the host sends the RESCIND_CHANNEL message, it doesn't > + * access the per-channel ringbuffer any longer. > + */ > + if (hdev->channel->rescind) > + return; > + > + /* Delete any children which might still exist. */ > + memset(&relations, 0, sizeof(relations)); > + hv_pci_devices_present(hbus, &relations); > + > + ret = hv_send_resources_released(hdev); > + if (ret) > + dev_err(&hdev->device, > + "Couldn't send resources released packet(s)\n"); > > memset(&pkt.teardown_packet, 0, sizeof(pkt.teardown_packet)); > init_completion(&comp_pkt.host_event); > @@ -2298,7 +2306,19 @@ static int hv_pci_remove(struct hv_device *hdev) > > VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); > if (!ret) > wait_for_completion_timeout(&comp_pkt.host_event, 10 * > HZ); > +} > + > +/** > + * hv_pci_remove() - Remove routine for this VMBus channel > + * @hdev:VMBus's tracking struct for this root PCI bus > + * > + * Return: 0 on success, -errno on failure > + */ > +static int hv_pci_remove(struct hv_device *hdev) > +{ > + struct hv_pcibus_device *hbus; > > + hbus = hv_get_drvdata(hdev); > if (hbus->state == hv_pcibus_installed) { > /* Remove the bus from PCI's point of view. */ > pci_lock_rescan_remove(); > @@ -2307,17 +2327,10 @@ static int hv_pci_remove(struct hv_device > *hdev) > pci_unlock_rescan_remove(); > } > > - ret = hv_send_resources_released(hdev); > - if (ret) > - dev_err(&hdev->device, > - "Couldn't send resources released packet(s)\n"); > + hv_pci_bus_exit(hdev); > > vmbus_close(hdev->channel); > > - /* Delete any children which might still exist. */ > - memset(&relations, 0, sizeof(relations)); > - hv_pci_devices_present(hbus, &relations); > - > iounmap(hbus->cfg_addr); > hv_free_config_window(hbus); > pci_free_resource_list(&hbus->resources_for_children); > -- > 2.7.4 This looks fine to me. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 3/3] PCI: hv: delete the device earlier from hbus->children for hot-remove
> -Original Message- > From: Dexuan Cui > Sent: Wednesday, November 9, 2016 11:20 PM > To: Bjorn Helgaas ; linux-...@vger.kernel.org; > de...@linuxdriverproject.org > Cc: gre...@linuxfoundation.org; KY Srinivasan ; > Haiyang Zhang ; Stephen Hemminger > ; Jake Oshins ; Hadden > Hoppert ; Vitaly Kuznetsov > ; jasow...@redhat.com; a...@canonical.com; > o...@aepfle.de; linux-ker...@vger.kernel.org > Subject: [PATCH 3/3] PCI: hv: delete the device earlier from hbus->children > for hot-remove > > After we send a PCI_EJECTION_COMPLETE message to the host, the host will > immediately send us a PCI_BUS_RELATIONS message with > relations->device_count == 0, so pci_devices_present_work(), running on > another thread, can find the being-ejected device, mark > the hpdev->reported_missing to true, and run list_move_tail()/list_del() > for the device -- this races hv_eject_device_work() -> list_del(). > > The patch moves the list_del() in hv_eject_device_work() to an earlier > place, i.e. before we send PCI_EJECTION_COMPLETE, so later the > pci_devices_present_work() can't see the device. > > Signed-off-by: Dexuan Cui > CC: Jake Oshins > Cc: KY Srinivasan > CC: Haiyang Zhang > CC: Vitaly Kuznetsov > --- > drivers/pci/host/pci-hyperv.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c > index 7590ad0..fe5179d 100644 > --- a/drivers/pci/host/pci-hyperv.c > +++ b/drivers/pci/host/pci-hyperv.c > @@ -1582,6 +1582,10 @@ static void hv_eject_device_work(struct > work_struct *work) > pci_dev_put(pdev); > } > > + spin_lock_irqsave(&hpdev->hbus->device_list_lock, flags); > + list_del(&hpdev->list_entry); > + spin_unlock_irqrestore(&hpdev->hbus->device_list_lock, flags); > + > memset(&ctxt, 0, sizeof(ctxt)); > ejct_pkt = (struct pci_eject_response *)&ctxt.pkt.message; > ejct_pkt->message_type.type = PCI_EJECTION_COMPLETE; > @@ -1590,10 +1594,6 @@ static void hv_eject_device_work(struct > work_struct *work) >sizeof(*ejct_pkt), (unsigned long)&ctxt.pkt, >VM_PKT_DATA_INBAND, 0); > > - spin_lock_irqsave(&hpdev->hbus->device_list_lock, flags); > - list_del(&hpdev->list_entry); > - spin_unlock_irqrestore(&hpdev->hbus->device_list_lock, flags); > - > put_pcichild(hpdev, hv_pcidev_ref_childlist); > put_pcichild(hpdev, hv_pcidev_ref_pnp); > put_hvpcibus(hpdev->hbus); > -- > 2.7.4 This looks good, too. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3 4/7] PCI: Record an fwnode associated with root PCI buses, optionally
> -Original Message- > From: Jiang Liu [mailto:jiang@linux.intel.com] > Sent: Tuesday, October 27, 2015 12:10 AM > To: Jake Oshins ; gre...@linuxfoundation.org; KY > Srinivasan ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; t...@linutronix.de; Haiyang Zhang > ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Subject: Re: [PATCH v3 4/7] PCI: Record an fwnode associated with root PCI > buses, optionally > > On 2015/10/27 7:15, ja...@microsoft.com wrote: > > From: Jake Oshins > > > > This patch allows a PCI front-end implementation to supply an > fwnode_handle > > associated with a root PCI bus, optionally. If supplied, the PCI driver > > records this. > > > > This patch supports the next patch in the series, which looks up an IRQ > domain > > through this handle. > Hi JaKeo, > Instead of changing the pci_create_root_bus() interface, > how about packing fwnode into sysdata, then we may > either 1) introduce a helper to retrieve fwnode from sysdata > or 2) set host_bridge->fwnode = sysdata in function > pcibios_root_bridge_prepare. > > Thanks, > Gerry > Thanks for the review. I'll work up a version that uses this suggestion and resend. (I'll respond to your feedback on the other patches, too.) -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v3 7/7] PCI: hv: New paravirtual PCI front-end for Hyper-V VMs
> -Original Message- > From: Jiang Liu [mailto:jiang@linux.intel.com] > Sent: Tuesday, October 27, 2015 12:11 AM > To: Jake Oshins ; gre...@linuxfoundation.org; KY > Srinivasan ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; t...@linutronix.de; Haiyang Zhang > ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Subject: Re: [PATCH v3 7/7] PCI: hv: New paravirtual PCI front-end for Hyper- > V VMs > > On 2015/10/27 7:15, ja...@microsoft.com wrote: > > From: Jake Oshins > > (snip) > > +/** > > + * hv_pcie_init_irq_domain() - Initialize IRQ domain > > + * @hbus: The root PCI bus > > + * > > + * Return: '0' on success and error value on failure > > + */ > > +static int hv_pcie_init_irq_domain(struct hv_pcibus_device *hbus) > > +{ > > + hbus->msi_info.chip = &hv_msi_irq_chip; > > + hbus->msi_info.chip_data = hbus; > > + hbus->msi_info.ops = &hv_msi_ops; > > + hbus->msi_info.flags = (MSI_FLAG_USE_DEF_DOM_OPS | > > + MSI_FLAG_USE_DEF_CHIP_OPS | > MSI_FLAG_MULTI_PCI_MSI | > > + MSI_FLAG_PCI_MSIX); > When interrupt remapping is not supported, x86 vector allocator > can't support multiple MSI because it can't allocate continuous > vectors yet. So please confirm whether we could enable > MSI_FLAG_MULTI_PCI_MSI for HV. > We can actually handle the remapping in the hypervisor. I'll add a comment to that effect. > > + hbus->msi_info.handler = handle_edge_irq; > > + hbus->msi_info.handler_name = "edge"; > > + hbus->msi_info.data = hbus; > How about using following pattern so we could avoid exporting > too many interfaces? > > struct irq_domain *parent_domain = NULL; > hbus->msi_info.chip = &hv_msi_irq_chip; > hbus->msi_info.ops = &hv_msi_ops; > // Let arch code to fill in default ops for chip and domain > x86_setup_default_msi_irqdomian_info(&hbus->msi_info, > &parent_domain); > // Override default ops if not applicable > hbus->irq_domain = pci_msi_create_irq_domain(hbus->fwnode, >&hbus->msi_info, >parent_domain); > I understand your point here, but I'm having trouble making it play out. When I look at this, the only functions or structures which are supplied straight from exports (in my proposed patches) are irq_chip_ack_parent(), pci_msi_prepare() and x86_vector_domain. The other exports either already exist for other reasons or they're needed within functions that I need to supply. If you feel strongly that adding a new function to avoid exporting these is the right way to go, I'll do it, but I want to confirm that first. Thanks, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v4 6/7] drivers:hv: Define the channel type for Hyper-V PCI Express pass-through
> -Original Message- > From: Andy Shevchenko [mailto:andy.shevche...@gmail.com] > Sent: Thursday, October 29, 2015 5:45 PM > To: Jake Oshins > Cc: Greg Kroah-Hartman ; KY Srinivasan > ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; Vitaly > Kuznetsov ; t...@redhat.com; Haiyang Zhang > ; marc.zyng...@arm.com; Jiang Liu > ; Bjorn Helgaas ; linux- > p...@vger.kernel.org > Subject: Re: [PATCH v4 6/7] drivers:hv: Define the channel type for Hyper-V > PCI Express pass-through > > > > > /* > > + * PCI Express Pass Through > > + * {44C4F61D--4400-9D52-802E27EDE19F} > > + */ > > + > > +#define HV_PCIE_GUID \ > > + .guid = { \ > > + 0x1D, 0xF6, 0xC4, 0x44, 0x44, 0x44, 0x00, 0x44, \ > > + 0x9D, 0x52, 0x80, 0x2E, 0x27, 0xED, 0xE1, 0x9F \ > > + } > > What about > #include > > UUID_LE(...) > > And you may use uuid_le type instead of custom raw buffer. > Thanks. I was just following the form of all the others in this file. I think that this was done to match up with the representation that's coming across the VM boundary from Windows. I do very much appreciate the feedback, though, Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v4 5/7] PCI: irqdomain: Look up IRQ domain by fwnode_handle
> -Original Message- > From: Andy Shevchenko [mailto:andy.shevche...@gmail.com] > Sent: Thursday, October 29, 2015 5:28 PM > To: Jake Oshins > Cc: Greg Kroah-Hartman ; KY Srinivasan > ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; Vitaly > Kuznetsov ; t...@redhat.com; Haiyang Zhang > ; marc.zyng...@arm.com; Jiang Liu > ; Bjorn Helgaas ; linux- > p...@vger.kernel.org > Subject: Re: [PATCH v4 5/7] PCI: irqdomain: Look up IRQ domain by > fwnode_handle > > On Fri, Oct 30, 2015 at 1:46 AM, wrote: > > From: Jake Oshins > > > > This patch adds a second way of finding an IRQ domain associated with > > a root PCI bus. After looking to see if one can be found through > > the OF tree, it attempts to look up the IRQ domain through an > > fwnode_handle stored in the pci_sysdata struct. > > > > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > > + /* > > +* If no IRQ domain was found via the OF tree, try looking it up > > +* directly through the fwnode_handle. > > +*/ > > + if (!d) { > > + if (pci_fwnode(bus)) { > > Isn't it the same to > if (!d && pci_fwnode(bus)) > d = ... > Thanks. I'll make this change and resend. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v4 4/7] PCI: Add fwnode_handle to pci_sysdata
> -Original Message- > From: Jiang Liu [mailto:jiang@linux.intel.com] > Sent: Thursday, October 29, 2015 6:55 PM > To: Jake Oshins ; gre...@linuxfoundation.org; KY > Srinivasan ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > vkuzn...@redhat.com; t...@redhat.com; Haiyang Zhang > ; marc.zyng...@arm.com; > bhelg...@google.com; linux-...@vger.kernel.org > Subject: Re: [PATCH v4 4/7] PCI: Add fwnode_handle to pci_sysdata > > > > > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > > +static inline void *pci_fwnode(struct pci_bus *bus) > > +{ > > + struct pci_sysdata *sd = bus->sysdata; > > + return sd->fwnode; > > +} > > +#endif > Hi Jakeo, > I would be better that if the function name indicates > that we are getting PCI host bridge(root bus) firmware node. > And you also need some magic here to avoid breaking compilation > on other archs: > in arch/x86/include/asm/pci.h > #define pci_fwnodepci_fwnode > > in include/asm-generic/pci.h > #ifndef pci_fwnode > #define pci_fwnode(bus) ((void)(bus),NULL) > #endif > > Thanks, > Gerry Got it. Thanks. I'll fix and resend. -- Jake Oshins ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH v5 5/7] PCI: irqdomain: Look up IRQ domain by fwnode_handle
> -Original Message- > From: Andy Shevchenko [mailto:andy.shevche...@gmail.com] > Sent: Friday, October 30, 2015 2:44 PM > To: Jake Oshins > Cc: Greg Kroah-Hartman ; KY Srinivasan > ; linux-ker...@vger.kernel.org; > de...@linuxdriverproject.org; o...@aepfle.de; Robo Bot > ; Vitaly Kuznetsov ; > t...@redhat.com; Haiyang Zhang ; > marc.zyng...@arm.com; Bjorn Helgaas ; linux- > p...@vger.kernel.org > Subject: Re: [PATCH v5 5/7] PCI: irqdomain: Look up IRQ domain by > fwnode_handle > > On Fri, Oct 30, 2015 at 11:11 PM, wrote: > > From: Jake Oshins > > > > This patch adds a second way of finding an IRQ domain associated with > > a root PCI bus. After looking to see if one can be found through > > the OF tree, it attempts to look up the IRQ domain through an > > fwnode_handle stored in the pci_sysdata struct. > > > > Signed-off-by: Jake Oshins > > --- > > arch/x86/include/asm/pci.h | 4 +++- > > drivers/pci/probe.c| 11 +++ > > include/asm-generic/pci.h | 4 > > 3 files changed, 18 insertions(+), 1 deletion(-) > > > > diff --git a/arch/x86/include/asm/pci.h b/arch/x86/include/asm/pci.h > > index 10213a1..fb74453 100644 > > --- a/arch/x86/include/asm/pci.h > > +++ b/arch/x86/include/asm/pci.h > > @@ -45,11 +45,13 @@ static inline int pci_proc_domain(struct pci_bus > *bus) > > #endif > > > > #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > > -static inline void *pci_fwnode(struct pci_bus *bus) > > +static inline void *_pci_root_bus_fwnode(struct pci_bus *bus) > > I'm sorry what is the point to rename? > I renamed it only because Gerry asked me to rename it. And then I regenerated the patch series with that rename still in the queue, which was a mistake. The whole thing is introduced in the previous patch. pci_fwnode() doesn't exist without this patch series. I'll straighten that out. > > { > > struct pci_sysdata *sd = bus->sysdata; > > return sd->fwnode; > > } > > + > > +#define pci_root_bus_fwnode_pci_root_bus_fwnode > > #endif > > > > /* Can be used to override the logic in pci_scan_bus for skipping > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > > index f441d1b..60e50a8 100644 > > --- a/drivers/pci/probe.c > > +++ b/drivers/pci/probe.c > > @@ -671,6 +671,17 @@ static struct irq_domain > *pci_host_bridge_msi_domain(struct pci_bus *bus) > > */ > > d = pci_host_bridge_of_msi_domain(bus); > > > > +#ifdef CONFIG_PCI_MSI_IRQ_DOMAIN > > + /* > > +* If no IRQ domain was found via the OF tree, try looking it up > > +* directly through the fwnode_handle. > > +*/ > > + if (!d && pci_root_bus_fwnode(bus)) { > > + d = irq_find_matching_fwnode(pci_root_bus_fwnode(bus), > > +DOMAIN_BUS_PCI_MSI); > > Gave a second glance and now noticed that you call > pci_root_bus_fwnode() twice. So, it actually might be more readable > like your first but modified variant: > > if (!d) { > void *fwnode = pci_fwnode(bus); > > if (fwnode) > d = irq_find_matching_fwnode(fwnode, DOMAIN_BUS_PCI_MSI); > } > > > + } > > +#endif > > + > > return d; > > } > > > With Best Regards, > Andy Shevchenko Sure. No problem. I'll resend. -- Jake ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging/skein: rename files and clean up directory structure
On Sun, 18 May 2014 11:42:53 -0700 Joe Perches wrote: > On Sun, 2014-05-18 at 12:36 -0600, Jake Edge wrote: > > Clean up file names and locations. Get rid of include/ directory > > and move those up to the top-level. Rename files to get rid of > > upper case. Remove skeinBlockNo3F.c as it was unused (temporary > > file or something?). > [] > > Diff appears to be huge because of the rearranging, not that much > > to it, really ... > > Hi Jake. > > Please use "git format-patch -M" when renaming files. > > This patch becomes simple and much more legible. Ah, so it does, thank you! I'll send a v2 here in a second ... jake -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/1] staging/skein: rename files and clean up directory structure
Clean up file names and locations. Get rid of include/ directory and move those up to the top-level. Rename files to get rid of upper case. Remove skeinBlockNo3F.c as it was unused (temporary file or something?). Signed-off-by: Jake Edge --- Against next-20140516 v2: use git format-patch -M as suggested by Joe Perches drivers/staging/skein/Makefile | 12 +- drivers/staging/skein/TODO | 1 - drivers/staging/skein/skein.c | 6 +- drivers/staging/skein/{include => }/skein.h| 0 drivers/staging/skein/skeinBlockNo3F.c | 175 - drivers/staging/skein/{skeinApi.c => skein_api.c} | 2 +- .../skein/{include/skeinApi.h => skein_api.h} | 2 +- drivers/staging/skein/skein_block.c| 2 +- drivers/staging/skein/{include => }/skein_block.h | 2 +- drivers/staging/skein/{include => }/skein_iv.h | 2 +- ...threefish1024Block.c => threefish_1024_block.c} | 2 +- .../{threefish256Block.c => threefish_256_block.c} | 2 +- .../{threefish512Block.c => threefish_512_block.c} | 2 +- .../skein/{threefishApi.c => threefish_api.c} | 4 +- .../{include/threefishApi.h => threefish_api.h}| 2 +- 15 files changed, 18 insertions(+), 198 deletions(-) rename drivers/staging/skein/{include => }/skein.h (100%) delete mode 100644 drivers/staging/skein/skeinBlockNo3F.c rename drivers/staging/skein/{skeinApi.c => skein_api.c} (99%) rename drivers/staging/skein/{include/skeinApi.h => skein_api.h} (99%) rename drivers/staging/skein/{include => }/skein_block.h (92%) rename drivers/staging/skein/{include => }/skein_iv.h (98%) rename drivers/staging/skein/{threefish1024Block.c => threefish_1024_block.c} (99%) rename drivers/staging/skein/{threefish256Block.c => threefish_256_block.c} (99%) rename drivers/staging/skein/{threefish512Block.c => threefish_512_block.c} (99%) rename drivers/staging/skein/{threefishApi.c => threefish_api.c} (98%) rename drivers/staging/skein/{include/threefishApi.h => threefish_api.h} (99%) diff --git a/drivers/staging/skein/Makefile b/drivers/staging/skein/Makefile index 2bb386e..395454c 100644 --- a/drivers/staging/skein/Makefile +++ b/drivers/staging/skein/Makefile @@ -1,13 +1,11 @@ # # Makefile for the skein secure hash algorithm # -subdir-ccflags-y := -I$(src)/include/ - obj-$(CONFIG_CRYPTO_SKEIN) += skein.o \ - skeinApi.o \ + skein_api.o \ skein_block.o -obj-$(CONFIG_CRYPTO_THREEFISH) += threefish1024Block.o \ - threefish256Block.o \ - threefish512Block.o \ - threefishApi.o +obj-$(CONFIG_CRYPTO_THREEFISH) += threefish_1024_block.o \ + threefish_256_block.o \ + threefish_512_block.o \ + threefish_api.o diff --git a/drivers/staging/skein/TODO b/drivers/staging/skein/TODO index f5c167a..1a4ce28 100644 --- a/drivers/staging/skein/TODO +++ b/drivers/staging/skein/TODO @@ -2,7 +2,6 @@ skein/threefish TODO - rename camelcase vars - rename camelcase functions - - rename files - move macros into appropriate header files - add / pass test vectors - module support diff --git a/drivers/staging/skein/skein.c b/drivers/staging/skein/skein.c index 096b86b..77cfedd 100644 --- a/drivers/staging/skein/skein.c +++ b/drivers/staging/skein/skein.c @@ -11,9 +11,9 @@ #define SKEIN_PORT_CODE /* instantiate any code in skein_port.h */ #include/* get the memcpy/memset functions */ -#include /* get the Skein API definitions */ -#include /* get precomputed IVs */ -#include +#include "skein.h" /* get the Skein API definitions */ +#include "skein_iv.h"/* get precomputed IVs */ +#include "skein_block.h" /*/ /* 256-bit Skein */ diff --git a/drivers/staging/skein/include/skein.h b/drivers/staging/skein/skein.h similarity index 100% rename from drivers/staging/skein/include/skein.h rename to drivers/staging/skein/skein.h diff --git a/drivers/staging/skein/skeinBlockNo3F.c b/drivers/staging/skein/skeinBlockNo3F.c deleted file mode 100644 index 6917638..000 --- a/drivers/staging/skein/skeinBlockNo3F.c +++ /dev/null @@ -1,175 +0,0 @@ - -#include -#include -#include - - -/* Skein_256 **/ -void Skein_256_Process_Block(struct skein_256_ctx *ctx, const u8 *blkPtr, - size_t blkCnt, size_t byteCntAdd) -{ - struct threefish_key key; - u64 tweak[2]; - int i; - u64 w[SKEIN_256_STATE_WORDS]; /* local copy of input block */
Re: [PATCH v2 1/1] staging/skein: rename files and clean up directory structure
On Sun, 18 May 2014 18:52:31 -0400 Jason Cooper wrote: > Jake, would you mind dropping this patch and perhaps assisting me with > reviewing Anton's series? I'll have him resend it as-is and we can go > from there. No, that's fine. Guess I'll put my patches to clean up some other stuff (function names, etc.) on hold and see how far Anton got ... fwiw, I did look at the lists to see if someone had already been working on this stuff, which is why it is such a good idea make sure the patches get posted publicly ... jake -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 06/06] staging: crypto: skein: rename files
On Mon, 19 May 2014 12:09:59 +0400 Anton Saraev wrote: > rename drivers/staging/skein/{skeinBlockNo3F.c => > skein_block_no_3f.c} (99%) fwiw, this file (skeinBlockNo3F.c) seems to be unused ... it kind of looks like a temporary file that got checked in by mistake. It's probably worth deleting it in this patch or a separate one, it seems to me. jake -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 06/06] staging: crypto: skein: rename files
On Mon, 19 May 2014 12:09:59 +0400 Anton Saraev wrote: > --- a/drivers/staging/skein/Makefile > +++ b/drivers/staging/skein/Makefile > @@ -4,10 +4,10 @@ > subdir-ccflags-y := -I$(src)/include/ as part of the rearranging of files, I think you want to get rid of this include directory, move the files up to the main directory and include them as "skein.h" (rather than ). Then this line can be removed from the Makefile. > --- a/drivers/staging/skein/threefish1024Block.c > +++ b/drivers/staging/skein/threefish_1024_block.c > @@ -1,5 +1,5 @@ > #include > -#include > +#include incidentally, none of the threefish_*_block.c files actually needs linux/string.h ... there may be other files where that's true too, it seems to have just gotten added everywhere ... jake -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 02/06] staging: crypto: skein: rename camelcase vars
On Mon, 19 May 2014 12:09:55 +0400 Anton Saraev wrote: > struct skein_ctx_hdr { > - size_t hashBitLen; /* size of hash result, in bits */ > - size_t bCnt; /* current byte count in buffer b[] */ > - u64 T[SKEIN_MODIFIER_WORDS]; /* tweak: T[0]=byte cnt, T[1]=flags */ > + size_t hash_bit_len;/* size of hash result, in bits */ > + size_t b_cnt; /* current byte count in buffer b[] */ > + u64 T[SKEIN_MODIFIER_WORDS];/* tweak: T[0]=byte cnt, T[1]=flags */ seems like 'T' should be lower case ... maybe renamed too (tweak?)? > struct skein_256_ctx { /* 256-bit Skein hash context structure */ > struct skein_ctx_hdr h; /* common header context variables */ > - u64 X[SKEIN_256_STATE_WORDS]; /* chaining variables */ > - u8 b[SKEIN_256_BLOCK_BYTES]; /* partial block buf (8-byte aligned) */ > + u64 X[SKEIN_256_STATE_WORDS]; /* chaining variables */ > + u8 b[SKEIN_256_BLOCK_BYTES];/* partial block buf (8-byte aligned) */ > }; > > struct skein_512_ctx { /* 512-bit Skein hash context structure */ > struct skein_ctx_hdr h; /* common header context variables */ > - u64 X[SKEIN_512_STATE_WORDS]; /* chaining variables */ > - u8 b[SKEIN_512_BLOCK_BYTES]; /* partial block buf (8-byte aligned) */ > + u64 X[SKEIN_512_STATE_WORDS]; /* chaining variables */ > + u8 b[SKEIN_512_BLOCK_BYTES];/* partial block buf (8-byte aligned) */ > }; > > struct skein1024_ctx { /* 1024-bit Skein hash context structure */ > struct skein_ctx_hdr h; /* common header context variables */ > - u64 X[SKEIN1024_STATE_WORDS]; /* chaining variables */ > - u8 b[SKEIN1024_BLOCK_BYTES]; /* partial block buf (8-byte aligned) */ > + u64 X[SKEIN1024_STATE_WORDS]; /* chaining variables */ > + u8 b[SKEIN1024_BLOCK_BYTES];/* partial block buf (8-byte aligned) */ the X arrays here should be renamed too, I suspect ... > @@ -99,8 +99,8 @@ enum skein_size { > * structures as well. > */ > struct skein_ctx { > - u64 skeinSize; > - u64 XSave[SKEIN_MAX_STATE_WORDS]; /* save area for state variables */ > + u64 skein_size; > + u64 X_save[SKEIN_MAX_STATE_WORDS]; /* save area for state variables */ why not x_save? > -int skein_init(struct skein_ctx *ctx, size_t hashBitLen) > +int skein_init(struct skein_ctx *ctx, size_t hash_bit_len) > { > int ret = SKEIN_FAIL; > - size_t Xlen = 0; > + size_t X_len = 0; > u64 *X = NULL; maybe I am missing something, but why not 'x' and 'x_len' ? > -int skein_mac_init(struct skein_ctx *ctx, const u8 *key, size_t keyLen, > -size_t hashBitLen) > +int skein_mac_init(struct skein_ctx *ctx, const u8 *key, size_t key_len, > +size_t hash_bit_len) > { > int ret = SKEIN_FAIL; > u64 *X = NULL; > - size_t Xlen = 0; > - u64 treeInfo = SKEIN_CFG_TREE_INFO_SEQUENTIAL; > + size_t X_len = 0; > + u64 tree_info = SKEIN_CFG_TREE_INFO_SEQUENTIAL; and the same here? > void skein_reset(struct skein_ctx *ctx) > { > - size_t Xlen = 0; > + size_t X_len = 0; > u64 *X = NULL; here too ... > -void skein_256_process_block(struct skein_256_ctx *ctx, const u8 *blkPtr, > - size_t blkCnt, size_t byteCntAdd) > +void skein_256_process_block(struct skein_256_ctx *ctx, const u8 *blk_ptr, > + size_t blk_cnt, size_t byte_cnt_add) > { /* do it in C */ > enum { > WCNT = SKEIN_256_STATE_WORDS > @@ -66,10 +66,11 @@ void skein_256_process_block(struct skein_256_ctx *ctx, > const u8 *blkPtr, > u64 X0, X1, X2, X3; /* local copy of context vars, for speed */ > u64 w[WCNT]; /* local copy of input block */ > #ifdef SKEIN_DEBUG > - const u64 *Xptr[4]; /* use for debugging (help cc put Xn in regs) */ > - Xptr[0] = &X0; Xptr[1] = &X1; Xptr[2] = &X2; Xptr[3] = &X3; > + const u64 *X_ptr[4]; /* use for debugging (help cc put Xn in regs) */ > + > + X_ptr[0] = &X0; X_ptr[1] = &X1; X_ptr[2] = &X2; X_ptr[3] = &X3; bunch of Xs in through here too ... is X special somehow? maybe so because it is used in all that loop-unrolling gunk? jake -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 02/06] staging: crypto: skein: rename camelcase vars
On Mon, 19 May 2014 22:21:12 +0300 Dan Carpenter wrote: > So, Jake, you're right, but if he redoes patch 2/6 then he has to redo > the rest as well. The bad names were bad in the original code already, > so normally we let people fix things up in follow on patches in that > case. > > Is that ok with you? Sure, that seems fine ... I only had fairly minor quibbles, the vast majority of it is good stuff ... Not sure it's useful (or even the right way to go about it), but, if it is, Greg (or Jason or whoever) can add my: Reviewed-by: Jake Edge to the patch set ... jake -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/1] staging/skein: rename files and clean up directory structure
Clean up file names and locations. Get rid of include/ directory and move those up to the top-level. Rename files to get rid of upper case. Remove skeinBlockNo3F.c as it was unused (temporary file or something?). Signed-off-by: Jake Edge --- v3: against staging-next branch of staging tree v2: use git format-patch -M as suggested by Joe Perches drivers/staging/skein/Makefile | 12 +- drivers/staging/skein/TODO | 1 - drivers/staging/skein/skein.c | 6 +- drivers/staging/skein/{include => }/skein.h| 0 drivers/staging/skein/skeinBlockNo3F.c | 175 - drivers/staging/skein/{skeinApi.c => skein_api.c} | 2 +- .../skein/{include/skeinApi.h => skein_api.h} | 6 +- drivers/staging/skein/skein_block.c| 2 +- drivers/staging/skein/{include => }/skein_block.h | 2 +- drivers/staging/skein/{include => }/skein_iv.h | 2 +- ...threefish1024Block.c => threefish_1024_block.c} | 2 +- .../{threefish256Block.c => threefish_256_block.c} | 2 +- .../{threefish512Block.c => threefish_512_block.c} | 2 +- .../skein/{threefishApi.c => threefish_api.c} | 4 +- .../{include/threefishApi.h => threefish_api.h}| 4 +- 15 files changed, 21 insertions(+), 201 deletions(-) rename drivers/staging/skein/{include => }/skein.h (100%) delete mode 100644 drivers/staging/skein/skeinBlockNo3F.c rename drivers/staging/skein/{skeinApi.c => skein_api.c} (99%) rename drivers/staging/skein/{include/skeinApi.h => skein_api.h} (99%) rename drivers/staging/skein/{include => }/skein_block.h (92%) rename drivers/staging/skein/{include => }/skein_iv.h (98%) rename drivers/staging/skein/{threefish1024Block.c => threefish_1024_block.c} (99%) rename drivers/staging/skein/{threefish256Block.c => threefish_256_block.c} (99%) rename drivers/staging/skein/{threefish512Block.c => threefish_512_block.c} (99%) rename drivers/staging/skein/{threefishApi.c => threefish_api.c} (98%) rename drivers/staging/skein/{include/threefishApi.h => threefish_api.h} (99%) diff --git a/drivers/staging/skein/Makefile b/drivers/staging/skein/Makefile index 2bb386e..395454c 100644 --- a/drivers/staging/skein/Makefile +++ b/drivers/staging/skein/Makefile @@ -1,13 +1,11 @@ # # Makefile for the skein secure hash algorithm # -subdir-ccflags-y := -I$(src)/include/ - obj-$(CONFIG_CRYPTO_SKEIN) += skein.o \ - skeinApi.o \ + skein_api.o \ skein_block.o -obj-$(CONFIG_CRYPTO_THREEFISH) += threefish1024Block.o \ - threefish256Block.o \ - threefish512Block.o \ - threefishApi.o +obj-$(CONFIG_CRYPTO_THREEFISH) += threefish_1024_block.o \ + threefish_256_block.o \ + threefish_512_block.o \ + threefish_api.o diff --git a/drivers/staging/skein/TODO b/drivers/staging/skein/TODO index 88a6e81..cd3508d 100644 --- a/drivers/staging/skein/TODO +++ b/drivers/staging/skein/TODO @@ -1,6 +1,5 @@ skein/threefish TODO - - rename files - move macros into appropriate header files - add / pass test vectors - module support diff --git a/drivers/staging/skein/skein.c b/drivers/staging/skein/skein.c index 2a2da98..f76d585 100644 --- a/drivers/staging/skein/skein.c +++ b/drivers/staging/skein/skein.c @@ -11,9 +11,9 @@ #define SKEIN_PORT_CODE /* instantiate any code in skein_port.h */ #include/* get the memcpy/memset functions */ -#include /* get the Skein API definitions */ -#include /* get precomputed IVs */ -#include +#include "skein.h" /* get the Skein API definitions */ +#include "skein_iv.h"/* get precomputed IVs */ +#include "skein_block.h" /*/ /* 256-bit Skein */ diff --git a/drivers/staging/skein/include/skein.h b/drivers/staging/skein/skein.h similarity index 100% rename from drivers/staging/skein/include/skein.h rename to drivers/staging/skein/skein.h diff --git a/drivers/staging/skein/skeinBlockNo3F.c b/drivers/staging/skein/skeinBlockNo3F.c deleted file mode 100644 index 4ee7f9f..000 --- a/drivers/staging/skein/skeinBlockNo3F.c +++ /dev/null @@ -1,175 +0,0 @@ - -#include -#include -#include - - -/* Skein_256 **/ -void skein_256_process_block(struct skein_256_ctx *ctx, const u8 *blk_ptr, -size_t blk_cnt, size_t byte_cnt_add) -{ - struct threefish_key key; - u64 tweak[2]; - int i; - u64 w[SKEIN_256_STATE_WORDS]; /* local copy of input block */ - u64 words[3]; - - ske
[PATCH 0/3] staging/skein: more cleanup
Clean up a few more things in skein to get it closer to mainline inclusion. The first may be questionable (so I probably should have put it last -- oh well, I can always respin), but it seemed like putting all of the threefish block functions in one file, like the skein block functions are all in one file, made sense. Jake Edge (3): move all threefish block functions to one file, remove unneeded include fix some comment typos Rename a few more variables and structure member names to lower case. drivers/staging/skein/Makefile |4 +- drivers/staging/skein/skein.c| 148 +- drivers/staging/skein/skein.h| 34 +- drivers/staging/skein/skein_api.c| 32 +- drivers/staging/skein/skein_api.h|2 +- drivers/staging/skein/skein_block.c | 155 +- drivers/staging/skein/threefish_1024_block.c | 4902 --- drivers/staging/skein/threefish_256_block.c | 1139 drivers/staging/skein/threefish_512_block.c | 2225 --- drivers/staging/skein/threefish_api.h| 18 +- drivers/staging/skein/threefish_block.c | 8258 ++ 11 files changed, 8454 insertions(+), 8463 deletions(-) delete mode 100644 drivers/staging/skein/threefish_1024_block.c delete mode 100644 drivers/staging/skein/threefish_256_block.c delete mode 100644 drivers/staging/skein/threefish_512_block.c create mode 100644 drivers/staging/skein/threefish_block.c -- 1.9.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging/skein: comment typos
fix some comment typos Signed-off-by: Jake Edge --- against staging-next branch of staging tree drivers/staging/skein/threefish_api.h | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/staging/skein/threefish_api.h b/drivers/staging/skein/threefish_api.h index 2fce154..8d5ddf8 100644 --- a/drivers/staging/skein/threefish_api.h +++ b/drivers/staging/skein/threefish_api.h @@ -12,7 +12,7 @@ * follow the openSSL design but at the same time take care of some Threefish * specific behaviour and possibilities. * - * These are the low level functions that deal with Threefisch blocks only. + * These are the low level functions that deal with Threefish blocks only. * Implementations for cipher modes such as ECB, CFB, or CBC may use these * functions. * @@ -77,9 +77,9 @@ void threefish_set_key(struct threefish_key *key_ctx, u64 *key_data, u64 *tweak); /** - * Encrypt Threefisch block (bytes). + * Encrypt Threefish block (bytes). * - * The buffer must have at least the same length (number of bits) aas the + * The buffer must have at least the same length (number of bits) as the * state size for this key. The function uses the first @c state_size bits * of the input buffer, encrypts them and stores the result in the output * buffer. @@ -95,9 +95,9 @@ void threefish_encrypt_block_bytes(struct threefish_key *key_ctx, u8 *in, u8 *out); /** - * Encrypt Threefisch block (words). + * Encrypt Threefish block (words). * - * The buffer must have at least the same length (number of bits) aas the + * The buffer must have at least the same length (number of bits) as the * state size for this key. The function uses the first @c state_size bits * of the input buffer, encrypts them and stores the result in the output * buffer. @@ -115,9 +115,9 @@ void threefish_encrypt_block_words(struct threefish_key *key_ctx, u64 *in, u64 *out); /** - * Decrypt Threefisch block (bytes). + * Decrypt Threefish block (bytes). * - * The buffer must have at least the same length (number of bits) aas the + * The buffer must have at least the same length (number of bits) as the * state size for this key. The function uses the first @c state_size bits * of the input buffer, decrypts them and stores the result in the output * buffer @@ -133,9 +133,9 @@ void threefish_decrypt_block_bytes(struct threefish_key *key_ctx, u8 *in, u8 *out); /** - * Decrypt Threefisch block (words). + * Decrypt Threefish block (words). * - * The buffer must have at least the same length (number of bits) aas the + * The buffer must have at least the same length (number of bits) as the * state size for this key. The function uses the first @c state_size bits * of the input buffer, encrypts them and stores the result in the output * buffer. -- 1.9.0 -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging/skein: variable/member name cleanup
Rename a few more variables and structure member names to lower case. Signed-off-by: Jake Edge --- against staging-next branch of staging tree drivers/staging/skein/skein.c | 148 +- drivers/staging/skein/skein.h | 34 drivers/staging/skein/skein_api.c | 32 drivers/staging/skein/skein_api.h | 2 +- drivers/staging/skein/skein_block.c | 155 ++-- 5 files changed, 186 insertions(+), 185 deletions(-) diff --git a/drivers/staging/skein/skein.c b/drivers/staging/skein/skein.c index f76d585..8cc8358 100644 --- a/drivers/staging/skein/skein.c +++ b/drivers/staging/skein/skein.c @@ -33,16 +33,16 @@ int skein_256_init(struct skein_256_ctx *ctx, size_t hash_bit_len) switch (hash_bit_len) { /* use pre-computed values, where available */ case 256: - memcpy(ctx->X, SKEIN_256_IV_256, sizeof(ctx->X)); + memcpy(ctx->x, SKEIN_256_IV_256, sizeof(ctx->x)); break; case 224: - memcpy(ctx->X, SKEIN_256_IV_224, sizeof(ctx->X)); + memcpy(ctx->x, SKEIN_256_IV_224, sizeof(ctx->x)); break; case 160: - memcpy(ctx->X, SKEIN_256_IV_160, sizeof(ctx->X)); + memcpy(ctx->x, SKEIN_256_IV_160, sizeof(ctx->x)); break; case 128: - memcpy(ctx->X, SKEIN_256_IV_128, sizeof(ctx->X)); + memcpy(ctx->x, SKEIN_256_IV_128, sizeof(ctx->x)); break; default: /* here if there is no precomputed IV value available */ @@ -63,11 +63,11 @@ int skein_256_init(struct skein_256_ctx *ctx, size_t hash_bit_len) /* compute the initial chaining values from config block */ /* zero the chaining variables */ - memset(ctx->X, 0, sizeof(ctx->X)); + memset(ctx->x, 0, sizeof(ctx->x)); skein_256_process_block(ctx, cfg.b, 1, SKEIN_CFG_STR_LEN); break; } - /* The chaining vars ctx->X are now initialized for hash_bit_len. */ + /* The chaining vars ctx->x are now initialized for hash_bit_len. */ /* Set up to process the data message portion of the hash (default) */ skein_start_new_type(ctx, MSG); /* T0=0, T1= MSG type */ @@ -89,25 +89,25 @@ int skein_256_init_ext(struct skein_256_ctx *ctx, size_t hash_bit_len, skein_assert_ret(hash_bit_len > 0, SKEIN_BAD_HASHLEN); skein_assert_ret(key_bytes == 0 || key != NULL, SKEIN_FAIL); - /* compute the initial chaining values ctx->X[], based on key */ + /* compute the initial chaining values ctx->x[], based on key */ if (key_bytes == 0) { /* is there a key? */ /* no key: use all zeroes as key for config block */ - memset(ctx->X, 0, sizeof(ctx->X)); + memset(ctx->x, 0, sizeof(ctx->x)); } else { /* here to pre-process a key */ - skein_assert(sizeof(cfg.b) >= sizeof(ctx->X)); + skein_assert(sizeof(cfg.b) >= sizeof(ctx->x)); /* do a mini-Init right here */ /* set output hash bit count = state size */ - ctx->h.hash_bit_len = 8*sizeof(ctx->X); + ctx->h.hash_bit_len = 8*sizeof(ctx->x); /* set tweaks: T0 = 0; T1 = KEY type */ skein_start_new_type(ctx, KEY); /* zero the initial chaining variables */ - memset(ctx->X, 0, sizeof(ctx->X)); + memset(ctx->x, 0, sizeof(ctx->x)); /* hash the key */ skein_256_update(ctx, key, key_bytes); /* put result into cfg.b[] */ skein_256_final_pad(ctx, cfg.b); - /* copy over into ctx->X[] */ - memcpy(ctx->X, cfg.b, sizeof(cfg.b)); + /* copy over into ctx->x[] */ + memcpy(ctx->x, cfg.b, sizeof(cfg.b)); } /* * build/process the config block, type == CONFIG (could be @@ -130,7 +130,7 @@ int skein_256_init_ext(struct skein_256_ctx *ctx, size_t hash_bit_len, /* compute the initial chaining values from config block */ skein_256_process_block(ctx, cfg.b, 1, SKEIN_CFG_STR_LEN); - /* The chaining vars ctx->X are now initialized */ + /* The chaining vars ctx->x are now initialized */ /* Set up to process the data message portion of the hash (default) */ skein_start_new_type(ctx, MSG); @@ -197,12 +197,12 @@ int skein_256_update(struct skein_256_ctx *ctx, const u8 *msg, int skein_256_final(struct skein_256_ctx *ctx, u8 *hash_val) { size_t i, n, byte_cnt; - u64 X[SKEIN_256_STATE_WORDS]; + u64 x[SKEIN_256_STATE_WORDS]; /* ca
Re: [PATCH 0/3] staging/skein: more cleanup
On Tue, 20 May 2014 10:47:57 -0400 Jason Cooper wrote: > Do you have any other series pending for this driver? No and I won't be doing anything else for the next couple of days -- some darn weekly edition to deal with :) It seems like most of the straightforward stuff has been dealt with at this point. That rats nest of ifdefs in skein_block.c needs attention, but some kind of tests are needed to ensure nothing breaks before digging into that ... jake -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/3] staging/skein: more cleanup
On Wed, 21 May 2014 01:52:17 +0400 Anton Saraev wrote: > On Tue, May 20, 2014 at 10:24:11AM -0600, Jake Edge wrote: > > On Tue, 20 May 2014 10:47:57 -0400 Jason Cooper wrote: > > > > but some kind of tests are needed to ensure nothing breaks before > > digging into that ... > > I have some test: slightly modified version of tests from > https://github.com/wernerd/Skein3Fish. It works as debugfs entry > and require some modification needed for module support > ("public" function must be extern). As I can understand Jason > has his own tests. That would be logical to share them but > I don't know where. well, it seems to me that we want tests that eventually can be added into the crypto test framework once skein moves out of staging and into crypto ... Jason's objdiff seems like it would be used as another development tool ... so do you have your tests anywhere that we can look at them? or, failing that, maybe you can just email me a copy off-list or something? do you have patches pending for skein? i might get a chance to hack on this some over the weekend, and we may as well try to avoid duplicating each other's efforts ... jake -- Jake Edge - LWN - j...@lwn.net - http://lwn.net ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel