* Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Correct vmx/dfp handling in both KVM and TCG cases
From: Alexander Graf @ 2011-10-24 17:55 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc, qemu-devel@nongnu.org Developers
In-Reply-To: <8DEA1BBB-904C-457E-8269-3A4C98092EC3@suse.de>
On 24.10.2011, at 10:25, Alexander Graf wrote:
>
> On 23.10.2011, at 22:29, David Gibson wrote:
>
>> On Thu, Oct 20, 2011 at 11:49:40PM -0700, Alexander Graf wrote:
>>>
>>> On 20.10.2011, at 22:06, David Gibson wrote:
>>>
>>>> On Thu, Oct 20, 2011 at 07:40:00PM -0700, Alexander Graf wrote:
>>>>> On 20.10.2011, at 17:41, David Gibson <david@gibson.dropbear.id.au> wrote:
>>>>>> On Thu, Oct 20, 2011 at 10:12:51AM -0700, Alexander Graf wrote:
>>>>>>> On 17.10.2011, at 21:15, David Gibson wrote:
>>>> [snip]
>>>>>> So, I really don't follow what the logic you want is. It sounds more
>>>>>> like what I have already, so I'm not sure how -cpu host comes into
>>>>>> this.
>>>>>
>>>>> Well, I want something very simple, layered:
>>>>>
>>>>> -cpu host only searches for pvr matches and selects a different CPU
>>>>> -type based on this
>>>>
>>>> Hrm, ok, well I can do this if you like, but note that this is quite
>>>> different from how -cpu host behaves on x86. There it builds the CPU
>>>> spec from scratch based on querying the host cpuid, rather than
>>>> selecting from an existing list of cpus. I selected from the existing
>>>> table based on host PVR because that was the easiest source for some
>>>> of the info in the cpu_spec, but my intention was that anything we
>>>> _can_ query directly from the host would override the table.
>>>>
>>>> It seems to be your approach is giving up on the possibility of
>>>> allowing -cpu host to work (and give you full access to the host
>>>> features) when qemu doesn't recognize the precise PVR of the host cpu.
>>>
>>> I disagree :). This is what x86 does:
>>>
>>> * -cpu host fetches CPUID info from host, puts it into vcpu
>>> * vcpu CPUID info gets ANDed with KVM capability CPUIDs
>>>
>>> I want basically the same thing. I want to have 2 different layers
>>> for 2 different semantics. One for what the host CPU would be able
>>> to do and one for what we can emulate, and two different steps to
>>> ensure control over them.
>>>
>>> The thing I think I'm apparently not bringing over yet is that I'm
>>> more than happy to get rid of the PVR searching step for -cpu host
>>> and instead use a full host capability inquiry mechanism. But that
>>> inquiry should indicate what the host CPU can do. It has nothing to
>>> do with KVM yet. The masking with KVM capabilities should be the
>>> next separate step.
>>>
>>> My goal is really to separate different layers into actual different
>>> layers :).
>>
>> Hrm. I think I see what you're getting at. Although nothing in that
>> patch is about kvm capabilities - it's all about working out what the
>> host's cpu can do.
>
> Reading through the patch again I think I see your point now :). Yes, the kvmppc_host_cpu_def function only tries to fetch the host CPU capabilities.
>
> So yes, there is basically only the masking part with what we can actually virtualize missing. But for now we can just assume that every feature the host CPU supports is available.
>
> I'll apply your patch for now, as it certainly is better than what we had before.
This breaks on 970mp (PowerStation). kvmppc_get_vmx returns -1 because ibm,vmx doesn't exist in the host dt, but the CPU still supports Altivec.
Any alternative way to enumerate VMX availability?
Alex
^ permalink raw reply
* [PATCH v3 2/2] dp83640: free packet queues on remove
From: Richard Cochran @ 2011-10-24 17:55 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Johannes Berg, stable
In-Reply-To: <cover.1319478544.git.richard.cochran@omicron.at>
If the PHY should disappear (for example, on an USB Ethernet MAC), then
the driver would leak any undelivered time stamp packets. This commit
fixes the issue by calling the appropriate functions to free any packets
left in the transmit and receive queues.
The driver first appeared in v3.0.
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: <stable@vger.kernel.org>
---
drivers/net/phy/dp83640.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index 311f5cb..dc44b73 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -875,6 +875,7 @@ static void dp83640_remove(struct phy_device *phydev)
struct dp83640_clock *clock;
struct list_head *this, *next;
struct dp83640_private *tmp, *dp83640 = phydev->priv;
+ struct sk_buff *skb;
if (phydev->addr == BROADCAST_ADDR)
return;
@@ -882,6 +883,12 @@ static void dp83640_remove(struct phy_device *phydev)
enable_status_frames(phydev, false);
cancel_work_sync(&dp83640->ts_work);
+ while ((skb = skb_dequeue(&dp83640->rx_queue)) != NULL)
+ kfree_skb(skb);
+
+ while ((skb = skb_dequeue(&dp83640->tx_queue)) != NULL)
+ skb_complete_tx_timestamp(skb, NULL);
+
clock = dp83640_clock_get(dp83640->clock);
if (dp83640 == clock->chosen) {
--
1.7.2.5
^ permalink raw reply related
* [PATCH v3 1/2] dp83640: use proper function to free transmit time stamping packets
From: Richard Cochran @ 2011-10-24 17:55 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Johannes Berg, stable
In-Reply-To: <cover.1319478544.git.richard.cochran@omicron.at>
Commit da92b194 introduced a new rule for handling the cloned packets
for transmit time stamping. These packets must not be freed using any other
function than skb_complete_tx_timestamp. This commit fixes the one and only
driver using this API.
The driver first appeared in v3.0.
Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
Cc: <stable@vger.kernel.org>
---
drivers/net/phy/dp83640.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/phy/dp83640.c b/drivers/net/phy/dp83640.c
index edd7304..311f5cb 100644
--- a/drivers/net/phy/dp83640.c
+++ b/drivers/net/phy/dp83640.c
@@ -1060,7 +1060,7 @@ static void dp83640_txtstamp(struct phy_device *phydev,
struct dp83640_private *dp83640 = phydev->priv;
if (!dp83640->hwts_tx_en) {
- kfree_skb(skb);
+ skb_complete_tx_timestamp(skb, NULL);
return;
}
skb_queue_tail(&dp83640->tx_queue, skb);
--
1.7.2.5
^ permalink raw reply related
* [PATCH v3 0/2] net: time stamping fixes
From: Richard Cochran @ 2011-10-24 17:55 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Eric Dumazet, Johannes Berg
[ Changes in v3: omit accepted patch and fixup the dp83640 patches. ]
These two patches depend on commit da92b194 and fix two bugs in a
PTP Hardware Clock driver. This driver was first introduced in Linux
version 3.0.
Richard Cochran (2):
dp83640: use proper function to free transmit time stamping packets
dp83640: free packet queues on remove
drivers/net/phy/dp83640.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
--
1.7.2.5
^ permalink raw reply
* |PATCH net-next] tg3: add tx_dropped counter
From: Eric Dumazet @ 2011-10-24 17:53 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Matt Carlson, Michael Chan
If a frame cant be transmitted, it is silently discarded.
Add a counter to report these errors to user.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
Note : merge errors expected because of pending tg3 patch in net tree, I
can respin if needed.
drivers/net/ethernet/broadcom/tg3.c | 23 +++++++++++------------
drivers/net/ethernet/broadcom/tg3.h | 1 +
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
index b89027c..3447585 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -6671,10 +6671,8 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
u32 tcp_opt_len, hdr_len;
if (skb_header_cloned(skb) &&
- pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
- dev_kfree_skb(skb);
- goto out_unlock;
- }
+ pskb_expand_head(skb, 0, 0, GFP_ATOMIC))
+ goto drop;
iph = ip_hdr(skb);
tcp_opt_len = tcp_optlen(skb);
@@ -6746,10 +6744,9 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
len = skb_headlen(skb);
mapping = pci_map_single(tp->pdev, skb->data, len, PCI_DMA_TODEVICE);
- if (pci_dma_mapping_error(tp->pdev, mapping)) {
- dev_kfree_skb(skb);
- goto out_unlock;
- }
+ if (pci_dma_mapping_error(tp->pdev, mapping))
+ goto drop;
+
tnapi->tx_buffers[entry].skb = skb;
dma_unmap_addr_set(&tnapi->tx_buffers[entry], mapping, mapping);
@@ -6805,7 +6802,7 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
budget = tg3_tx_avail(tnapi);
if (tigon3_dma_hwbug_workaround(tnapi, skb, &entry, &budget,
base_flags, mss, vlan))
- goto out_unlock;
+ goto drop_nofree;
}
skb_tx_timestamp(skb);
@@ -6827,15 +6824,16 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
netif_tx_wake_queue(txq);
}
-out_unlock:
mmiowb();
-
return NETDEV_TX_OK;
dma_error:
tg3_tx_skb_unmap(tnapi, tnapi->tx_prod, i);
- dev_kfree_skb(skb);
tnapi->tx_buffers[tnapi->tx_prod].skb = NULL;
+drop:
+ dev_kfree_skb(skb);
+drop_nofree:
+ tp->tx_dropped++;
return NETDEV_TX_OK;
}
@@ -10009,6 +10007,7 @@ static struct rtnl_link_stats64 *tg3_get_stats64(struct net_device *dev,
get_stat64(&hw_stats->rx_discards);
stats->rx_dropped = tp->rx_dropped;
+ stats->tx_dropped = tp->tx_dropped;
return stats;
}
diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
index d2976f3..f32f288 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -2990,6 +2990,7 @@ struct tg3 {
/* begin "everything else" cacheline(s) section */
unsigned long rx_dropped;
+ unsigned long tx_dropped;
struct rtnl_link_stats64 net_stats_prev;
struct tg3_ethtool_stats estats;
struct tg3_ethtool_stats estats_prev;
^ permalink raw reply related
* Re: [linux-lvm] Changing dev_t-devname mapping in lvmlib seems to be problematic
From: Stuart D. Gathman @ 2011-10-24 17:52 UTC (permalink / raw)
To: LVM general discussion and development; +Cc: Zdenek Kabelac
In-Reply-To: <CAGRgLy5Sm_T19Ex+qLe=rTH3r_nSnYOhW+mU5HZy5Y1y2JnJAw@mail.gmail.com>
On Mon, 24 Oct 2011, Alexander Lyakas wrote:
>> dm-xxx devices are created dynamicaly - there is currently no way to
>> have a fixed dm device node and it would be quite ugly to provide such
>> feature.
> I did not understand this comment. Do you mean the dm-xxx devices that
> LVM creates for its LVs? I was talking about dm-linear devices that I
> use for PVs.
Why are you using dm-linear for PVs?
--
Stuart D. Gathman <stuart@bmsi.com>
Business Management Systems Inc. Phone: 703 591-0911 Fax: 703 591-6154
"Confutatis maledictis, flammis acribus addictis" - background song for
a Microsoft sponsored "Where do you want to go from here?" commercial.
^ permalink raw reply
* [PATCH] staging: comedi: Unbreak output of printk()s in pcmmio
From: Johannes Thumshirn @ 2011-10-24 17:52 UTC (permalink / raw)
To: gregkh; +Cc: linux-kernel, devel, morbidrsa, abbotti, fmhess, lucas.demarchi
In-Reply-To: <20111017221024.GA26595@kroah.com>
Unbreak the output of some printk()s I broke.
Signed-off-by: Johannes Thumshirn <morbidrsa@googlemail.com>
---
drivers/staging/comedi/drivers/pcmmio.c | 26 +++++++++++++++-----------
1 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmmio.c b/drivers/staging/comedi/drivers/pcmmio.c
index 3ad04aa..b9ca040 100644
--- a/drivers/staging/comedi/drivers/pcmmio.c
+++ b/drivers/staging/comedi/drivers/pcmmio.c
@@ -371,15 +371,15 @@ static int pcmmio_attach(struct comedi_device *dev, struct comedi_devconfig *it)
iobase = it->options[0];
irq[0] = it->options[1];
- printk(KERN_INFO "comedi%d: %s: io: %lx ", dev->minor, driver.driver_name,
- iobase);
+ printk(KERN_INFO "comedi%d: %s: io: %lx attaching...\n", dev->minor,
+ driver.driver_name, iobase);
dev->iobase = iobase;
if (!iobase || !request_region(iobase,
thisboard->total_iosize,
driver.driver_name)) {
- printk(KERN_ERR "I/O port conflict\n");
+ printk(KERN_ERR "comedi%d: I/O port conflict\n", dev->minor);
return -EIO;
}
@@ -394,7 +394,8 @@ static int pcmmio_attach(struct comedi_device *dev, struct comedi_devconfig *it)
* convenient macro defined in comedidev.h.
*/
if (alloc_private(dev, sizeof(struct pcmmio_private)) < 0) {
- printk(KERN_ERR "cannot allocate private data structure\n");
+ printk(KERN_ERR "comedi%d: cannot allocate private data structure\n",
+ dev->minor);
return -ENOMEM;
}
@@ -417,7 +418,8 @@ static int pcmmio_attach(struct comedi_device *dev, struct comedi_devconfig *it)
kcalloc(n_subdevs, sizeof(struct pcmmio_subdev_private),
GFP_KERNEL);
if (!devpriv->sprivs) {
- printk(KERN_ERR "cannot allocate subdevice private data structures\n");
+ printk(KERN_ERR "comedi%d: cannot allocate subdevice private data structures\n",
+ dev->minor);
return -ENOMEM;
}
/*
@@ -427,7 +429,8 @@ static int pcmmio_attach(struct comedi_device *dev, struct comedi_devconfig *it)
* Allocate 1 AI + 1 AO + 2 DIO subdevs (24 lines per DIO)
*/
if (alloc_subdevices(dev, n_subdevs) < 0) {
- printk(KERN_ERR "cannot allocate subdevice data structures\n");
+ printk(KERN_ERR "comedi%d: cannot allocate subdevice data structures\n",
+ dev->minor);
return -ENOMEM;
}
@@ -557,14 +560,15 @@ static int pcmmio_attach(struct comedi_device *dev, struct comedi_devconfig *it)
*/
if (irq[0]) {
- printk(KERN_DEBUG "irq: %u ", irq[0]);
+ printk(KERN_DEBUG "comedi%d: irq: %u\n", dev->minor, irq[0]);
if (thisboard->dio_num_asics == 2 && irq[1])
- printk(KERN_DEBUG "second ASIC irq: %u ", irq[1]);
+ printk(KERN_DEBUG "comedi%d: second ASIC irq: %u\n",
+ dev->minor, irq[1]);
} else {
- printk(KERN_INFO "(IRQ mode disabled) ");
+ printk(KERN_INFO "comedi%d: (IRQ mode disabled)\n", dev->minor);
}
- printk(KERN_INFO "attached\n");
+ printk(KERN_INFO "comedi%d: attached\n", dev->minor);
return 1;
}
@@ -663,7 +667,7 @@ static int pcmmio_dio_insn_bits(struct comedi_device *dev,
}
#ifdef DAMMIT_ITS_BROKEN
/* DEBUG */
- printk(KERN_DEBUG "data_out_byte %02x\n", (unsigned)byte);
+ printk("data_out_byte %02x\n", (unsigned)byte);
#endif
/* save the digital input lines for this byte.. */
s->state |= ((unsigned int)byte) << offset;
--
1.7.5.4
^ permalink raw reply related
* [Bug 42168] false reporting of GL_MAX_TEXTURE_SIZE for nvidia geforce 9800 gt
From: bugzilla-daemon-CC+yJ3UmIYqDUpFQwHEjaQ @ 2011-10-24 17:51 UTC (permalink / raw)
To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
In-Reply-To: <bug-42168-8800-V0hAGp6uBxMKqLRl/0Ahz6D7qz1kEfGD2LY78lusg7I@public.gmane.org/>
https://bugs.freedesktop.org/show_bug.cgi?id=42168
--- Comment #1 from drago01-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org 2011-10-24 10:51:52 PDT ---
Created attachment 52702
--> https://bugs.freedesktop.org/attachment.cgi?id=52702
[PATCH] Fix max texture levels on nv50
I have already sent it to the ml, but attaching here anyway.
--
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
^ permalink raw reply
* [PATCH] Fix max texture levels on nv50
From: Adel Gadllah @ 2011-10-24 17:50 UTC (permalink / raw)
To: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Hi,
The attached patch fixes the texture limits on nv50 to match what the
hardware is actually capable off.
-----
From: Adel Gadllah <adel.gadllah-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Mon, 24 Oct 2011 19:41:03 +0200
Subject: [PATCH] Fix max texture levels on nv50
MAX_TEXTURE_2D_LEVELS and MAX_TEXTURE_CUBE_LEVELS are supposed to be
14 not 13, while MAX_TEXTURE_3D_LEVELS should be 12 not 10.
---
src/gallium/drivers/nv50/nv50_screen.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/gallium/drivers/nv50/nv50_screen.c
b/src/gallium/drivers/nv50/nv50_screen.c
index 0bd6057..1270c83 100644
--- a/src/gallium/drivers/nv50/nv50_screen.c
+++ b/src/gallium/drivers/nv50/nv50_screen.c
@@ -80,11 +80,11 @@ nv50_screen_get_param(struct pipe_screen *pscreen,
enum pipe_cap param)
case PIPE_CAP_MAX_COMBINED_SAMPLERS:
return 64;
case PIPE_CAP_MAX_TEXTURE_2D_LEVELS:
- return 13;
+ return 14;
case PIPE_CAP_MAX_TEXTURE_3D_LEVELS:
- return 10;
+ return 12;
case PIPE_CAP_MAX_TEXTURE_CUBE_LEVELS:
- return 13;
+ return 14;
case PIPE_CAP_MAX_TEXTURE_ARRAY_LAYERS: /* shader support missing */
return 0;
case PIPE_CAP_MIN_TEXEL_OFFSET:
--
1.7.6.4
^ permalink raw reply related
* Re: Time for a release of 1.14.0?
From: Chris Larson @ 2011-10-24 17:40 UTC (permalink / raw)
To: Richard Purdie; +Cc: bitbake-devel
In-Reply-To: <1319474440.26563.19.camel@ted>
On Mon, Oct 24, 2011 at 9:40 AM, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> This is just a heads up that I think its time we had a release. Various
> factors (including but not limited to my push access to the repo being
> broken atm) have meant I've run out of time before ELC-E so this will
> likely happen at the start of next week unless there are major
> objections.
Sounds good to me, it's definitely a good time for it.
--
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics
^ permalink raw reply
* Re: [PATCH v2 2/3] dp83640: use proper function to free transmit time stamping packets
From: Richard Cochran @ 2011-10-24 17:47 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eric.dumazet, johannes, stable
In-Reply-To: <20111024.025555.265760947744724258.davem@davemloft.net>
On Mon, Oct 24, 2011 at 02:55:55AM -0400, David Miller wrote:
> From: Richard Cochran <richardcochran@gmail.com>
> Date: Fri, 21 Oct 2011 12:49:16 +0200
>
> > The previous commit enforces a new rule for handling the cloned packets
> > for transmit time stamping. These packets must not be freed using any other
> > function than skb_complete_tx_timestamp. This commit fixes the one and only
> > driver using this API.
> >
> > The driver first appeared in v3.0.
> >
> > Signed-off-by: Richard Cochran <richard.cochran@omicron.at>
> > Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
> > Cc: <stable@vger.kernel.org>
>
> In the 'net' tree, which is where you should be targetting these dp83640
> driver patches, the code looks nothing like what you're patching against.
>
> Please respin patches #2 and #3 against current sources.
Okay, but #2 will conflict with
dccaa9e0 dp83640: add time stamp insertion for sync messages
in net-next. Should I also submit a fix for that one?
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 06/11] ttm/driver: Expand ttm_backend_func to include two overrides for TTM page pool.
From: Thomas Hellstrom @ 2011-10-24 17:42 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: linux-kernel, dri-devel, thellstrom, airlied, xen-devel, j.glisse,
bskeggs
In-Reply-To: <20111024172728.GD2320@phenom.dumpdata.com>
On 10/24/2011 07:27 PM, Konrad Rzeszutek Wilk wrote:
> On Sat, Oct 22, 2011 at 11:40:54AM +0200, Thomas Hellstrom wrote:
>
>> Konrad,
>>
>> I was hoping that we could get rid of the dma_address shuffling into
>> core TTM,
>> like I mentioned in the review. From what I can tell it's now only
>> used in the backend and
>> core ttm doesn't care about it.
>>
>> Is there a particular reason we're still passing it around?
>>
> Yes - and I should have addressed that in the writeup but forgot, sorry about that.
>
> So initially I thought you meant this:
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> index 360afb3..06ef048 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> @@ -662,8 +662,7 @@ out:
>
> /* Put all pages in pages list to correct pool to wait for reuse */
> static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
> - int flags, enum ttm_caching_state cstate,
> - dma_addr_t *dma_address)
> + int flags, enum ttm_caching_state cstate)
> {
> unsigned long irq_flags;
> struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
> @@ -707,8 +706,7 @@ static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
> * cached pages.
> */
> static int __ttm_get_pages(struct list_head *pages, int flags,
> - enum ttm_caching_state cstate, unsigned count,
> - dma_addr_t *dma_address)
> + enum ttm_caching_state cstate, unsigned count)
> {
> struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
> struct page *p = NULL;
> @@ -864,7 +862,7 @@ int ttm_get_pages(struct ttm_tt *ttm, struct list_head *pages,
> if (ttm->be&& ttm->be->func&& ttm->be->func->get_pages)
> return ttm->be->func->get_pages(ttm, pages, count, dma_address);
> return __ttm_get_pages(pages, ttm->page_flags, ttm->caching_state,
> - count, dma_address);
> + count)
> }
> void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
> unsigned page_count, dma_addr_t *dma_address)
> @@ -873,5 +871,5 @@ void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
> ttm->be->func->put_pages(ttm, pages, page_count, dma_address);
> else
> __ttm_put_pages(pages, page_count, ttm->page_flags,
> - ttm->caching_state, dma_address);
> + ttm->caching_state)
> }
> which is trivial (thought I have not compile tested it), but it should do it.
>
> But I think you mean eliminate the dma_address handling completly in
> ttm_page_alloc.c and ttm_tt.c.
>
> For that there are couple of architectural issues I am not sure how to solve.
>
> There has to be some form of TTM<->[Radeon|Nouveau] lookup mechanism
> to say: "here is a 'struct page *', give me the bus address". Currently
> this is solved by keeping an array of DMA addresses along with the list
> of pages. And passing the list and DMA address up the stack (and down)
> from TTM up to the driver (when ttm->be->func->populate is called and they
> are handed off) does it. It does not break any API layering .. and the internal
> TTM pool (non-DMA) can just ignore the dma_address altogether (see patch above).
>
>
I actually had something more simple in mind, but when tinking a bit
deeper into it, it seems more complicated than I initially thought.
Namely that when we allocate pages from the ttm_backend, we actually
populated it at the same time. be::populate would then not take a page
array as an argument, and would actually be a no-op on many
drivers.
This makes us move towards struct ttm_tt consisting almost only of its
backend, so that whole API should perhaps be looked at with new eyes.
So anyway, I'm fine with high level things as they are now, and the
dma_addr issue can be looked at at a later time. If we could get a
couple of extra eyes to review the code for style etc. would be great,
because I have very little time the next couple of weeks.
/Thomas
^ permalink raw reply
* Re: [PATCH 06/11] ttm/driver: Expand ttm_backend_func to include two overrides for TTM page pool.
From: Thomas Hellstrom @ 2011-10-24 17:42 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: thellstrom, xen-devel, linux-kernel, dri-devel, j.glisse, airlied,
bskeggs
In-Reply-To: <20111024172728.GD2320@phenom.dumpdata.com>
On 10/24/2011 07:27 PM, Konrad Rzeszutek Wilk wrote:
> On Sat, Oct 22, 2011 at 11:40:54AM +0200, Thomas Hellstrom wrote:
>
>> Konrad,
>>
>> I was hoping that we could get rid of the dma_address shuffling into
>> core TTM,
>> like I mentioned in the review. From what I can tell it's now only
>> used in the backend and
>> core ttm doesn't care about it.
>>
>> Is there a particular reason we're still passing it around?
>>
> Yes - and I should have addressed that in the writeup but forgot, sorry about that.
>
> So initially I thought you meant this:
>
> diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> index 360afb3..06ef048 100644
> --- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
> +++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
> @@ -662,8 +662,7 @@ out:
>
> /* Put all pages in pages list to correct pool to wait for reuse */
> static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
> - int flags, enum ttm_caching_state cstate,
> - dma_addr_t *dma_address)
> + int flags, enum ttm_caching_state cstate)
> {
> unsigned long irq_flags;
> struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
> @@ -707,8 +706,7 @@ static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
> * cached pages.
> */
> static int __ttm_get_pages(struct list_head *pages, int flags,
> - enum ttm_caching_state cstate, unsigned count,
> - dma_addr_t *dma_address)
> + enum ttm_caching_state cstate, unsigned count)
> {
> struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
> struct page *p = NULL;
> @@ -864,7 +862,7 @@ int ttm_get_pages(struct ttm_tt *ttm, struct list_head *pages,
> if (ttm->be&& ttm->be->func&& ttm->be->func->get_pages)
> return ttm->be->func->get_pages(ttm, pages, count, dma_address);
> return __ttm_get_pages(pages, ttm->page_flags, ttm->caching_state,
> - count, dma_address);
> + count)
> }
> void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
> unsigned page_count, dma_addr_t *dma_address)
> @@ -873,5 +871,5 @@ void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
> ttm->be->func->put_pages(ttm, pages, page_count, dma_address);
> else
> __ttm_put_pages(pages, page_count, ttm->page_flags,
> - ttm->caching_state, dma_address);
> + ttm->caching_state)
> }
> which is trivial (thought I have not compile tested it), but it should do it.
>
> But I think you mean eliminate the dma_address handling completly in
> ttm_page_alloc.c and ttm_tt.c.
>
> For that there are couple of architectural issues I am not sure how to solve.
>
> There has to be some form of TTM<->[Radeon|Nouveau] lookup mechanism
> to say: "here is a 'struct page *', give me the bus address". Currently
> this is solved by keeping an array of DMA addresses along with the list
> of pages. And passing the list and DMA address up the stack (and down)
> from TTM up to the driver (when ttm->be->func->populate is called and they
> are handed off) does it. It does not break any API layering .. and the internal
> TTM pool (non-DMA) can just ignore the dma_address altogether (see patch above).
>
>
I actually had something more simple in mind, but when tinking a bit
deeper into it, it seems more complicated than I initially thought.
Namely that when we allocate pages from the ttm_backend, we actually
populated it at the same time. be::populate would then not take a page
array as an argument, and would actually be a no-op on many
drivers.
This makes us move towards struct ttm_tt consisting almost only of its
backend, so that whole API should perhaps be looked at with new eyes.
So anyway, I'm fine with high level things as they are now, and the
dma_addr issue can be looked at at a later time. If we could get a
couple of extra eyes to review the code for style etc. would be great,
because I have very little time the next couple of weeks.
/Thomas
^ permalink raw reply
* Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write
From: Marek Olšák @ 2011-10-24 17:39 UTC (permalink / raw)
To: Thomas Hellstrom, Dave Airlie; +Cc: dri-devel
In-Reply-To: <4EA5A040.8000307@shipmail.org>
Alright then.
Dave, if you are reading this, feel free not to include the two
patches I sent you in the next pull request.
Marek
On Mon, Oct 24, 2011 at 7:28 PM, Thomas Hellstrom <thomas@shipmail.org> wrote:
> Marek,
>
> The problem is that the patch adds a lot of complicated code where it's not
> needed, and I don't want to end up reverting that code and re-implementing
> the new Radeon gem ioctl by myself.
>
> Having a list of two fence objects and waiting for either of them shouldn't
> be that complicated to implement, in particular when it's done in a
> driver-specific way and you have the benefit of assuming that they are
> ordered.
>
> Since the new functionality is a performance improvement, If time is an
> issue, I suggest we back this change out and go for the next merge window.
>
> /Thomas
>
>
> On 10/24/2011 07:10 PM, Marek Olšák wrote:
>>
>> Hi Thomas,
>>
>> I have made no progress so far due to lack of time.
>>
>> Would you mind if I fixed the most important things first, which are:
>> - sync objects are not ordered, (A)
>> - every sync object must have its corresponding sync_obj_arg, (B)
>> and if I fixed (C) some time later.
>>
>> I planned on moving the two sync objects from ttm into radeon and not
>> using ttm_bo_wait from radeon (i.e. pretty much reimplementing what it
>> does), but it looks more complicated to me than I had originally
>> thought.
>>
>> Marek
>>
>> On Mon, Oct 24, 2011 at 4:48 PM, Thomas Hellstrom<thomas@shipmail.org>
>> wrote:
>>
>>>
>>> Marek,
>>> Any progress on this. The merge window is about to open soon I guess and
>>> we
>>> need a fix by then.
>>>
>>> /Thomas
>>>
>
>
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: kdump: crash_kexec()-smp_send_stop() race in panic
From: Vivek Goyal @ 2011-10-24 17:33 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Américo Wang, holzheu, akpm, schwidefsky, heiko.carstens,
kexec, linux-kernel
In-Reply-To: <m1aa8qfhdk.fsf@fess.ebiederm.org>
On Mon, Oct 24, 2011 at 10:07:19AM -0700, Eric W. Biederman wrote:
> Américo Wang <xiyou.wangcong@gmail.com> writes:
>
> > On Mon, Oct 24, 2011 at 11:14 PM, Eric W. Biederman
> > <ebiederm@xmission.com> wrote:
> >> Michael Holzheu <holzheu@linux.vnet.ibm.com> writes:
> >>
> >>> Hello Vivek,
> >>>
> >>> In our tests we ran into the following scenario:
> >>>
> >>> Two CPUs have called panic at the same time. The first CPU called
> >>> crash_kexec() and the second CPU called smp_send_stop() in panic()
> >>> before crash_kexec() finished on the first CPU. So the second CPU
> >>> stopped the first CPU and therefore kdump failed.
> >>>
> >>> 1st CPU:
> >>> panic()->crash_kexec()->mutex_trylock(&kexec_mutex)-> do kdump
> >>>
> >>> 2nd CPU:
> >>> panic()->crash_kexec()->kexec_mutex already held by 1st CPU
> >>> ->smp_send_stop()-> stop CPU 1 (stop kdump)
> >>>
> >>> How should we fix this problem? One possibility could be to do
> >>> smp_send_stop() before we call crash_kexec().
> >>>
> >>> What do you think?
> >>
> >> smp_send_stop is insufficiently reliable to be used before crash_kexec.
> >>
> >> My first reaction would be to test oops_in_progress and wait until
> >> oops_in_progress == 1 before calling smp_send_stop.
> >>
> >
> > +1
> >
> > One of my colleague mentioned the same problem with me inside
> > RH, given the fact that the race condition window is small, it would
> > not be easy to reproduce this scenario.
>
> As for reproducing it I have a hunch you could hack up something
> horrible with smp_call_function and kprobes.
>
>
> On a little more reflection we can't wait until oops_in_progress goes
> to 1 before calling smp_send_stop. Because if crash_kexec is not
> involved nothing we will never call smp_send_stop.
>
> So my second thought is to introduce another atomic variable
> panic_in_progress, visible only in panic. The cpu that sets
> increments panic_in_progress can call smp_send_stop. The rest of
> the cpus can just go into a busy wait. That should stop nasty
> fights about who is going to come out of smp_send_stop first.
Introducing panic_on_oops atomic sounds good.
Thanks
Vivek
^ permalink raw reply
* Re: kdump: crash_kexec()-smp_send_stop() race in panic
From: Vivek Goyal @ 2011-10-24 17:33 UTC (permalink / raw)
To: Eric W. Biederman
Cc: kexec, heiko.carstens, linux-kernel, schwidefsky,
Américo Wang, akpm, holzheu
In-Reply-To: <m1aa8qfhdk.fsf@fess.ebiederm.org>
On Mon, Oct 24, 2011 at 10:07:19AM -0700, Eric W. Biederman wrote:
> Américo Wang <xiyou.wangcong@gmail.com> writes:
>
> > On Mon, Oct 24, 2011 at 11:14 PM, Eric W. Biederman
> > <ebiederm@xmission.com> wrote:
> >> Michael Holzheu <holzheu@linux.vnet.ibm.com> writes:
> >>
> >>> Hello Vivek,
> >>>
> >>> In our tests we ran into the following scenario:
> >>>
> >>> Two CPUs have called panic at the same time. The first CPU called
> >>> crash_kexec() and the second CPU called smp_send_stop() in panic()
> >>> before crash_kexec() finished on the first CPU. So the second CPU
> >>> stopped the first CPU and therefore kdump failed.
> >>>
> >>> 1st CPU:
> >>> panic()->crash_kexec()->mutex_trylock(&kexec_mutex)-> do kdump
> >>>
> >>> 2nd CPU:
> >>> panic()->crash_kexec()->kexec_mutex already held by 1st CPU
> >>> ->smp_send_stop()-> stop CPU 1 (stop kdump)
> >>>
> >>> How should we fix this problem? One possibility could be to do
> >>> smp_send_stop() before we call crash_kexec().
> >>>
> >>> What do you think?
> >>
> >> smp_send_stop is insufficiently reliable to be used before crash_kexec.
> >>
> >> My first reaction would be to test oops_in_progress and wait until
> >> oops_in_progress == 1 before calling smp_send_stop.
> >>
> >
> > +1
> >
> > One of my colleague mentioned the same problem with me inside
> > RH, given the fact that the race condition window is small, it would
> > not be easy to reproduce this scenario.
>
> As for reproducing it I have a hunch you could hack up something
> horrible with smp_call_function and kprobes.
>
>
> On a little more reflection we can't wait until oops_in_progress goes
> to 1 before calling smp_send_stop. Because if crash_kexec is not
> involved nothing we will never call smp_send_stop.
>
> So my second thought is to introduce another atomic variable
> panic_in_progress, visible only in panic. The cpu that sets
> increments panic_in_progress can call smp_send_stop. The rest of
> the cpus can just go into a busy wait. That should stop nasty
> fights about who is going to come out of smp_send_stop first.
Introducing panic_on_oops atomic sounds good.
Thanks
Vivek
_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 5/5] qxl: support async monitor screen dump
From: Alon Levy @ 2011-10-24 17:30 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: qemu-devel, mlureau, armbru, lcapitulino
In-Reply-To: <4EA5846B.9080101@redhat.com>
On Mon, Oct 24, 2011 at 05:29:47PM +0200, Gerd Hoffmann wrote:
> On 10/24/11 14:02, Alon Levy wrote:
> > Split qxl_spice_update_area_complete from qxl_render_update, use
> > SPICE_INTERFACE_QXL_MINOR 2 introduced spice_qxl_update_area_dirty_async
> > to retrive the dirty rectangles asyncronously (the previous
> > spice_qxl_update_area_async did not accept a dirty rectangles array).
> >
> > Introduce SpiceAsyncMonitorScreenDump for a screen_dump.
>
> That one conflicts with the screendump/SDL fixes pushed to the spice.v44
> branch. Have you seen the mail? Had you time to look at the patches?
Yes and no. I will.
>
> cheers,
> Gerd
>
^ permalink raw reply
* Re: [Qemu-devel] [PATCH 1/5] monitor: screen_dump async
From: Alon Levy @ 2011-10-24 17:29 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: qemu-devel, mdroth, mlureau, Gerd Hoffmann, armbru
In-Reply-To: <20111024134516.57596735@doriath>
On Mon, Oct 24, 2011 at 01:45:16PM -0200, Luiz Capitulino wrote:
> On Mon, 24 Oct 2011 17:13:14 +0200
> Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > On 10/24/11 14:02, Alon Levy wrote:
> > > Make screen_dump monitor command an async command to allow next for qxl
> > > to implement it as a initiating call to red_worker and completion on
> > > callback, to fix a deadlock when issueing a screendump command via
> > > libvirt while connected with a libvirt controlled spice-gtk client.
> >
> > Approach looks reasonable to me. Patch breaks the build though, you've
> > missed a bunch of screen_dump functions in non-x86 targets.
>
> There are two problems actually.
>
> The first one is that changing an existing command from synchronous
> to asynchronous is an incompatible change because asynchronous commands
> semantics is different. For an example of possible problems please
> check: https://bugzilla.redhat.com/show_bug.cgi?id=623903.
>
> The second problem is that the existing asynchronous interface in the
> monitor is incomplete and has never been used for real. Our plan is to
> use QAPI's async support, but that has not landed in master yet and iirc
> there wasn't consensus about it. I also think it's a bit late for its
> inclusion in 1.0 (and certainly not a candidate for stable).
>
> If all you need here is to delay sending the response, then maybe the
> current interface could work (although I honestly don't trust it and
> regret not having dropped it). Otherwise our only choice would be to
> work on getting the QAPI async support merged.
My problem is that the io thread keeps the global mutex during the wait,
that's why the async monitor is perfect for what I want - this is
exactly what it does. I haven't looked at QAPI async support, but I
understand it's a bit in the future.
^ permalink raw reply
* Re: [PATCH 1/2] drm/ttm: add a way to bo_wait for either the last read or last write
From: Thomas Hellstrom @ 2011-10-24 17:28 UTC (permalink / raw)
To: Marek Olšák; +Cc: dri-devel
In-Reply-To: <CAAxE2A56Xgyx1ridKhaj8EtbVn-AcPM_BoQBQusVAk3gBKVCqQ@mail.gmail.com>
Marek,
The problem is that the patch adds a lot of complicated code where it's
not needed, and I don't want to end up reverting that code and
re-implementing the new Radeon gem ioctl by myself.
Having a list of two fence objects and waiting for either of them
shouldn't be that complicated to implement, in particular when it's done
in a driver-specific way and you have the benefit of assuming that they
are ordered.
Since the new functionality is a performance improvement, If time is an
issue, I suggest we back this change out and go for the next merge window.
/Thomas
On 10/24/2011 07:10 PM, Marek Olšák wrote:
> Hi Thomas,
>
> I have made no progress so far due to lack of time.
>
> Would you mind if I fixed the most important things first, which are:
> - sync objects are not ordered, (A)
> - every sync object must have its corresponding sync_obj_arg, (B)
> and if I fixed (C) some time later.
>
> I planned on moving the two sync objects from ttm into radeon and not
> using ttm_bo_wait from radeon (i.e. pretty much reimplementing what it
> does), but it looks more complicated to me than I had originally
> thought.
>
> Marek
>
> On Mon, Oct 24, 2011 at 4:48 PM, Thomas Hellstrom<thomas@shipmail.org> wrote:
>
>> Marek,
>> Any progress on this. The merge window is about to open soon I guess and we
>> need a fix by then.
>>
>> /Thomas
>>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* Re: [PATCH 05/10] user namespace: clamp down users of cap_raised
From: Serge E. Hallyn @ 2011-10-24 17:28 UTC (permalink / raw)
To: Andrew G. Morgan
Cc: Serge E. Hallyn, David Howells, linux-kernel, ebiederm, akpm,
oleg, richard, mikevs, segoon, gregkh, eparis
In-Reply-To: <CALQRfL4zCbzr_Fa0ySi96j06nw7ifocCxUjYUs6vXxeNvjCutQ@mail.gmail.com>
Quoting Andrew G. Morgan (morgan@kernel.org):
> Serge,
>
> It seems as if this whole thing is really idiomatic. How about?
>
> #define IN_ROOT_USER_NS_CAPABLE(cap) \
> ((current_user_ns() == &init_user_ns) && cap_raised(current_cap(), cap))
My objection to this was that it seems to encourage others to use it :) I'm
not sure we want that. Also, IN_ROOT_USER_NS seems more generally useful.
But if I'm the only one who feels this way I'll go ahead and do it...
thanks,
-serge
^ permalink raw reply
* Re: [PATCH 06/11] ttm/driver: Expand ttm_backend_func to include two overrides for TTM page pool.
From: Konrad Rzeszutek Wilk @ 2011-10-24 17:27 UTC (permalink / raw)
To: Thomas Hellstrom
Cc: linux-kernel, dri-devel, thellstrom, airlied, xen-devel, j.glisse,
bskeggs
In-Reply-To: <4EA28FA6.7000006@shipmail.org>
On Sat, Oct 22, 2011 at 11:40:54AM +0200, Thomas Hellstrom wrote:
> Konrad,
>
> I was hoping that we could get rid of the dma_address shuffling into
> core TTM,
> like I mentioned in the review. From what I can tell it's now only
> used in the backend and
> core ttm doesn't care about it.
>
> Is there a particular reason we're still passing it around?
Yes - and I should have addressed that in the writeup but forgot, sorry about that.
So initially I thought you meant this:
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 360afb3..06ef048 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -662,8 +662,7 @@ out:
/* Put all pages in pages list to correct pool to wait for reuse */
static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
- int flags, enum ttm_caching_state cstate,
- dma_addr_t *dma_address)
+ int flags, enum ttm_caching_state cstate)
{
unsigned long irq_flags;
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
@@ -707,8 +706,7 @@ static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
* cached pages.
*/
static int __ttm_get_pages(struct list_head *pages, int flags,
- enum ttm_caching_state cstate, unsigned count,
- dma_addr_t *dma_address)
+ enum ttm_caching_state cstate, unsigned count)
{
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
struct page *p = NULL;
@@ -864,7 +862,7 @@ int ttm_get_pages(struct ttm_tt *ttm, struct list_head *pages,
if (ttm->be && ttm->be->func && ttm->be->func->get_pages)
return ttm->be->func->get_pages(ttm, pages, count, dma_address);
return __ttm_get_pages(pages, ttm->page_flags, ttm->caching_state,
- count, dma_address);
+ count)
}
void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
unsigned page_count, dma_addr_t *dma_address)
@@ -873,5 +871,5 @@ void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
ttm->be->func->put_pages(ttm, pages, page_count, dma_address);
else
__ttm_put_pages(pages, page_count, ttm->page_flags,
- ttm->caching_state, dma_address);
+ ttm->caching_state)
}
which is trivial (thought I have not compile tested it), but it should do it.
But I think you mean eliminate the dma_address handling completly in
ttm_page_alloc.c and ttm_tt.c.
For that there are couple of architectural issues I am not sure how to solve.
There has to be some form of TTM<->[Radeon|Nouveau] lookup mechanism
to say: "here is a 'struct page *', give me the bus address". Currently
this is solved by keeping an array of DMA addresses along with the list
of pages. And passing the list and DMA address up the stack (and down)
from TTM up to the driver (when ttm->be->func->populate is called and they
are handed off) does it. It does not break any API layering .. and the internal
TTM pool (non-DMA) can just ignore the dma_address altogether (see patch above).
But if we wanted to rip all mention of dma_addr from TTM, one immediate way
that comes to my mind is:
1). Provide a new function in the ttm->be->func that would be called 'get_dma' of:
(int)( *get_dma)(struct list_head *pages, unsigned page_count, dma_addr_t *dma_address)
which would call the TTM DMA to search the internal list and find 'pages*'
(which were just a microsecond ago allocated by calling ttm->be->func->get_pages)
and stick the bus address on the 'dma_address' array.
2). The radeon|nouveau driver would both call this if they decided to use the
TTM DMA API. They would need to provide the newly allocated dma_address for this
call.
3). Not sure how to wrap this in macros though - it looks as if both drivers will
be riddled with 'if (ttm->be->func->get_pages) { private->dma_addr=kzalloc(...) } else {}'.
But that is more an implemention problem..
.. While this idea looks correct, I am struck that it looks like it is breaking the layering
of APIs, where the driver is reaching behind the TTM API and calling this extra function?
Another idea is to transform the 'struct dma_addr *dma_addr' to a 'void *override_p' in
the 'struct ttm_tt'. That means still keeping the TTM API layers seperate, and "passing"
the array of DMA address through the 'override_p' array (which would be allocated by TTM DMA
code). Something along these lines (not tested):
I like this more, but I haven't actually tested it so not sure if it works right?
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index e0d4474..8760a04 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -23,7 +23,7 @@ struct nouveau_sgdma_be {
static int
nouveau_sgdma_populate(struct ttm_backend *be, unsigned long num_pages,
struct page **pages, struct page *dummy_read_page,
- dma_addr_t *dma_addrs)
+ void *override_p)
{
struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)be;
struct drm_device *dev = nvbe->dev;
@@ -43,9 +43,9 @@ nouveau_sgdma_populate(struct ttm_backend *be, unsigned long num_pages,
nvbe->nr_pages = 0;
while (num_pages--) {
- if (dma_addrs[nvbe->nr_pages] != 0) {
- nvbe->pages[nvbe->nr_pages] =
- dma_addrs[nvbe->nr_pages];
+ dma_addr_t *ttm_dma = (dma_addr_t *)override_p;
+ if (ttm_dma && ttm_dma[nvbe->nr_pages] != 0) {
+ nvbe->pages[nvbe->nr_pages] = ttm_dma[nvbe->nr_pages];
nvbe->ttm_alloced[nvbe->nr_pages] = true;
} else {
nvbe->pages[nvbe->nr_pages] =
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 068ba09..dc700f4 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -181,7 +181,7 @@ int radeon_gart_bind(struct radeon_device *rdev, unsigned offset,
p = t / (PAGE_SIZE / RADEON_GPU_PAGE_SIZE);
for (i = 0; i < pages; i++, p++) {
- if (dma_addr[i] != 0) {
+ if (dma_addr && dma_addr[i] != 0) {
rdev->gart.ttm_alloced[p] = true;
rdev->gart.pages_addr[p] = dma_addr[i];
} else {
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 2e7419f..690545d 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -661,7 +661,7 @@ struct radeon_ttm_backend {
unsigned long num_pages;
struct page **pages;
struct page *dummy_read_page;
- dma_addr_t *dma_addrs;
+ dma_addr_t *dma_addrs; /* Can be NULL */
bool populated;
bool bound;
unsigned offset;
@@ -671,13 +671,13 @@ static int radeon_ttm_backend_populate(struct ttm_backend *backend,
unsigned long num_pages,
struct page **pages,
struct page *dummy_read_page,
- dma_addr_t *dma_addrs)
+ void *override_p)
{
struct radeon_ttm_backend *gtt;
gtt = container_of(backend, struct radeon_ttm_backend, backend);
gtt->pages = pages;
- gtt->dma_addrs = dma_addrs;
+ gtt->dma_addrs = (dma_addr_t *)override_p;
gtt->num_pages = num_pages;
gtt->dummy_read_page = dummy_read_page;
gtt->populated = true;
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 360afb3..458727a 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -662,8 +662,7 @@ out:
/* Put all pages in pages list to correct pool to wait for reuse */
static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
- int flags, enum ttm_caching_state cstate,
- dma_addr_t *dma_address)
+ int flags, enum ttm_caching_state cstate)
{
unsigned long irq_flags;
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
@@ -707,8 +706,7 @@ static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
* cached pages.
*/
static int __ttm_get_pages(struct list_head *pages, int flags,
- enum ttm_caching_state cstate, unsigned count,
- dma_addr_t *dma_address)
+ enum ttm_caching_state cstate, unsigned count)
{
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
struct page *p = NULL;
@@ -766,7 +764,7 @@ static int __ttm_get_pages(struct list_head *pages, int flags,
printk(KERN_ERR TTM_PFX
"Failed to allocate extra pages "
"for large request.");
- __ttm_put_pages(pages, 0, flags, cstate, NULL);
+ __ttm_put_pages(pages, 0, flags, cstate);
return r;
}
}
@@ -859,19 +857,19 @@ int ttm_page_alloc_debugfs(struct seq_file *m, void *data)
}
EXPORT_SYMBOL(ttm_page_alloc_debugfs);
int ttm_get_pages(struct ttm_tt *ttm, struct list_head *pages,
- unsigned count, dma_addr_t *dma_address)
+ unsigned count, int index)
{
if (ttm->be && ttm->be->func && ttm->be->func->get_pages)
- return ttm->be->func->get_pages(ttm, pages, count, dma_address);
+ return ttm->be->func->get_pages(ttm, pages, count, index);
return __ttm_get_pages(pages, ttm->page_flags, ttm->caching_state,
- count, dma_address);
+ count);
}
void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
- unsigned page_count, dma_addr_t *dma_address)
+ unsigned page_count, int index)
{
if (ttm->be && ttm->be->func && ttm->be->func->put_pages)
- ttm->be->func->put_pages(ttm, pages, page_count, dma_address);
+ ttm->be->func->put_pages(ttm, pages, page_count, index);
else
__ttm_put_pages(pages, page_count, ttm->page_flags,
- ttm->caching_state, dma_address);
+ ttm->caching_state);
}
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index a5be62e..08e182f 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -45,6 +45,7 @@
#include <linux/atomic.h>
#include <linux/device.h>
#include <linux/kthread.h>
+#include <drm/drm_mem_util.h>
#include "ttm/ttm_bo_driver.h"
#include "ttm/ttm_page_alloc.h"
#ifdef TTM_HAS_AGP
@@ -1097,7 +1098,7 @@ out:
* cached pages. On failure will hold the negative return value (-ENOMEM, etc).
*/
int ttm_dma_get_pages(struct ttm_tt *ttm, struct list_head *pages,
- unsigned count, dma_addr_t *dma_address)
+ unsigned count, int idx)
{
int r = -ENOMEM;
@@ -1105,6 +1106,11 @@ int ttm_dma_get_pages(struct ttm_tt *ttm, struct list_head *pages,
gfp_t gfp_flags;
enum pool_type type;
struct device *dev = ttm->be->dev;
+ dma_addr_t *dma_address;
+
+ /* We _MUST_ have a proper index value. */
+ if (WARN_ON(idx < 0));
+ return -EINVAL;
type = ttm_to_type(ttm->page_flags, ttm->caching_state);
@@ -1129,6 +1135,7 @@ int ttm_dma_get_pages(struct ttm_tt *ttm, struct list_head *pages,
cstate);
}
#endif
+ dma_address = &((dma_addr_t *)ttm->override_p)[idx];
/* Take pages out of a pool (if applicable) */
r = ttm_dma_pool_get_pages(pool, pages, dma_address, count);
/* clear the pages coming from the pool if requested */
@@ -1201,7 +1208,7 @@ static int ttm_dma_pool_get_num_unused_pages(void)
/* Put all pages in pages list to correct pool to wait for reuse */
void ttm_dma_put_pages(struct ttm_tt *ttm, struct list_head *pages,
- unsigned page_count, dma_addr_t *dma_address)
+ unsigned page_count, int idx)
{
struct dma_pool *pool;
enum pool_type type;
@@ -1230,9 +1237,12 @@ void ttm_dma_put_pages(struct ttm_tt *ttm, struct list_head *pages,
count = ttm_dma_put_pages_in_pool(pool, pages, page_count, is_cached);
- for (i = 0; i < count; i++)
- dma_address[i] = 0;
-
+ /* Optional. */
+ if (idx >= 0) {
+ dma_addr_t *dma_address = &((dma_addr_t *)ttm->override_p)[idx];
+ for (i = 0; i < count; i++)
+ dma_address[i] = 0;
+ }
spin_lock_irqsave(&pool->lock, irq_flags);
pool->npages_in_use -= count;
if (is_cached)
@@ -1386,11 +1396,27 @@ int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data)
return 0;
}
EXPORT_SYMBOL_GPL(ttm_dma_page_alloc_debugfs);
+
+static int ttm_dma_alloc_priv(struct ttm_tt *ttm)
+{
+ ttm->override_p = drm_calloc_large(ttm->num_pages, sizeof(dma_addr_t *));
+ if (WARN_ON(!ttm->override_p))
+ return -ENOMEM;
+ return 0;
+
+}
+static void ttm_dma_free_priv(struct ttm_tt *ttm)
+{
+ drm_free_large(ttm->override_p);
+ ttm->override_p = NULL;
+}
bool ttm_dma_override(struct ttm_backend_func *be)
{
if (swiotlb_nr_tbl() && be && !ttm_dma_disable) {
be->get_pages = &ttm_dma_get_pages;
be->put_pages = &ttm_dma_put_pages;
+ be->alloc_priv = &ttm_dma_alloc_priv;
+ be->free_priv = &ttm_dma_free_priv;
return true;
}
return false;
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 31ae359..0f0d57f 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -50,16 +50,16 @@ static int ttm_tt_swapin(struct ttm_tt *ttm);
static void ttm_tt_alloc_page_directory(struct ttm_tt *ttm)
{
ttm->pages = drm_calloc_large(ttm->num_pages, sizeof(*ttm->pages));
- ttm->dma_address = drm_calloc_large(ttm->num_pages,
- sizeof(*ttm->dma_address));
+ if (ttm->be && ttm->be->func && ttm->be->func->alloc_priv)
+ ttm->be->func->alloc_priv(ttm);
}
static void ttm_tt_free_page_directory(struct ttm_tt *ttm)
{
drm_free_large(ttm->pages);
ttm->pages = NULL;
- drm_free_large(ttm->dma_address);
- ttm->dma_address = NULL;
+ if (ttm->be && ttm->be->func && ttm->be->func->free_priv)
+ ttm->be->func->free_priv(ttm);
}
static void ttm_tt_free_user_pages(struct ttm_tt *ttm)
@@ -110,7 +110,7 @@ static struct page *__ttm_tt_get_page(struct ttm_tt *ttm, int index)
INIT_LIST_HEAD(&h);
- ret = ttm_get_pages(ttm, &h, 1, &ttm->dma_address[index]);
+ ret = ttm_get_pages(ttm, &h, 1, index);
if (ret != 0)
return NULL;
@@ -169,7 +169,7 @@ int ttm_tt_populate(struct ttm_tt *ttm)
}
be->func->populate(be, ttm->num_pages, ttm->pages,
- ttm->dummy_read_page, ttm->dma_address);
+ ttm->dummy_read_page, ttm->override_p);
ttm->state = tt_unbound;
return 0;
}
@@ -303,7 +303,7 @@ static void ttm_tt_free_alloced_pages(struct ttm_tt *ttm, bool call_clear)
count++;
}
}
- ttm_put_pages(ttm, &h, count, ttm->dma_address);
+ ttm_put_pages(ttm, &h, count, 0 /* start at zero and go up to count */);
ttm->state = tt_unpopulated;
ttm->first_himem_page = ttm->num_pages;
ttm->last_lomem_page = -1;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 1826c3b..771697a 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -58,7 +58,7 @@ struct ttm_backend_func {
int (*populate) (struct ttm_backend *backend,
unsigned long num_pages, struct page **pages,
struct page *dummy_read_page,
- dma_addr_t *dma_addrs);
+ void *override_p);
/**
* struct ttm_backend_func member clear
*
@@ -109,10 +109,11 @@ struct ttm_backend_func {
*
* @ttm: ttm which contains flags for page allocation and caching state.
* @pages: head of empty linked list where pages are filled.
- * @dma_address: The DMA (bus) address of pages
+ * @idx: The current index in ttm->pages[] array. Negative means
+ * don't assume ttm->pages[idx] order matches the order in *pages.
*/
int (*get_pages) (struct ttm_tt *ttm, struct list_head *pages,
- unsigned count, dma_addr_t *dma_address);
+ unsigned count, int idx);
/**
* ttm_put_pages override. The backend can override the default
@@ -124,10 +125,17 @@ struct ttm_backend_func {
* @pages: list of pages to free.
* @page_count: number of pages in the list. Zero can be passed for
* unknown count.
- * @dma_address: The DMA (bus) address of pages
+ * @idx: The current index in the ttm->pages[] array. Negative means
+ * don't assume ttm->pages[idx] order matches the order in *pages.
*/
void (*put_pages) (struct ttm_tt *ttm, struct list_head *pages,
- unsigned page_count, dma_addr_t *dma_address);
+ unsigned page_count, int idx);
+
+ /**
+ * TODO: Flesh this out.
+ */
+ int (*alloc_priv) (struct ttm_tt *ttm);
+ void (*free_priv) (struct ttm_tt *ttm);
};
/**
@@ -207,7 +215,7 @@ struct ttm_tt {
tt_unbound,
tt_unpopulated,
} state;
- dma_addr_t *dma_address;
+ void *override_p;
};
#define TTM_MEMTYPE_FLAG_FIXED (1 << 0) /* Fixed (on-card) PCI memory */
diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h
index daf5db6..31e6079 100644
--- a/include/drm/ttm/ttm_page_alloc.h
+++ b/include/drm/ttm/ttm_page_alloc.h
@@ -68,12 +68,12 @@ static inline int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data)
* @ttm: ttm which contains flags for page allocation and caching state.
* @pages: heado of empty linked list where pages are filled.
* @count: number of pages to allocate.
- * @dma_address: The DMA (bus) address of pages - (by default zero).
+ * @idx: The current index in ttm->pages[idx]. Negative means ignore.
*/
int ttm_get_pages(struct ttm_tt *ttm,
struct list_head *pages,
unsigned count,
- dma_addr_t *dma_address);
+ int idx);
/**
* Put linked list of pages to pool.
*
@@ -81,12 +81,12 @@ int ttm_get_pages(struct ttm_tt *ttm,
* @pages: list of pages to free.
* @page_count: number of pages in the list. Zero can be passed for unknown
* count.
- * @dma_address: The DMA (bus) address of pages (by default zero).
+ * @idx: The current index in ttm->pages[idx]. Negative means ignore.
*/
void ttm_put_pages(struct ttm_tt *ttm,
struct list_head *pages,
unsigned page_count,
- dma_addr_t *dma_address);
+ int idx);
/**
* Initialize pool allocator.
*/
^ permalink raw reply related
* Re: [PATCH 06/11] ttm/driver: Expand ttm_backend_func to include two overrides for TTM page pool.
From: Konrad Rzeszutek Wilk @ 2011-10-24 17:27 UTC (permalink / raw)
To: Thomas Hellstrom
Cc: thellstrom, xen-devel, linux-kernel, dri-devel, j.glisse, airlied,
bskeggs
In-Reply-To: <4EA28FA6.7000006@shipmail.org>
On Sat, Oct 22, 2011 at 11:40:54AM +0200, Thomas Hellstrom wrote:
> Konrad,
>
> I was hoping that we could get rid of the dma_address shuffling into
> core TTM,
> like I mentioned in the review. From what I can tell it's now only
> used in the backend and
> core ttm doesn't care about it.
>
> Is there a particular reason we're still passing it around?
Yes - and I should have addressed that in the writeup but forgot, sorry about that.
So initially I thought you meant this:
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 360afb3..06ef048 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -662,8 +662,7 @@ out:
/* Put all pages in pages list to correct pool to wait for reuse */
static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
- int flags, enum ttm_caching_state cstate,
- dma_addr_t *dma_address)
+ int flags, enum ttm_caching_state cstate)
{
unsigned long irq_flags;
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
@@ -707,8 +706,7 @@ static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
* cached pages.
*/
static int __ttm_get_pages(struct list_head *pages, int flags,
- enum ttm_caching_state cstate, unsigned count,
- dma_addr_t *dma_address)
+ enum ttm_caching_state cstate, unsigned count)
{
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
struct page *p = NULL;
@@ -864,7 +862,7 @@ int ttm_get_pages(struct ttm_tt *ttm, struct list_head *pages,
if (ttm->be && ttm->be->func && ttm->be->func->get_pages)
return ttm->be->func->get_pages(ttm, pages, count, dma_address);
return __ttm_get_pages(pages, ttm->page_flags, ttm->caching_state,
- count, dma_address);
+ count)
}
void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
unsigned page_count, dma_addr_t *dma_address)
@@ -873,5 +871,5 @@ void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
ttm->be->func->put_pages(ttm, pages, page_count, dma_address);
else
__ttm_put_pages(pages, page_count, ttm->page_flags,
- ttm->caching_state, dma_address);
+ ttm->caching_state)
}
which is trivial (thought I have not compile tested it), but it should do it.
But I think you mean eliminate the dma_address handling completly in
ttm_page_alloc.c and ttm_tt.c.
For that there are couple of architectural issues I am not sure how to solve.
There has to be some form of TTM<->[Radeon|Nouveau] lookup mechanism
to say: "here is a 'struct page *', give me the bus address". Currently
this is solved by keeping an array of DMA addresses along with the list
of pages. And passing the list and DMA address up the stack (and down)
from TTM up to the driver (when ttm->be->func->populate is called and they
are handed off) does it. It does not break any API layering .. and the internal
TTM pool (non-DMA) can just ignore the dma_address altogether (see patch above).
But if we wanted to rip all mention of dma_addr from TTM, one immediate way
that comes to my mind is:
1). Provide a new function in the ttm->be->func that would be called 'get_dma' of:
(int)( *get_dma)(struct list_head *pages, unsigned page_count, dma_addr_t *dma_address)
which would call the TTM DMA to search the internal list and find 'pages*'
(which were just a microsecond ago allocated by calling ttm->be->func->get_pages)
and stick the bus address on the 'dma_address' array.
2). The radeon|nouveau driver would both call this if they decided to use the
TTM DMA API. They would need to provide the newly allocated dma_address for this
call.
3). Not sure how to wrap this in macros though - it looks as if both drivers will
be riddled with 'if (ttm->be->func->get_pages) { private->dma_addr=kzalloc(...) } else {}'.
But that is more an implemention problem..
.. While this idea looks correct, I am struck that it looks like it is breaking the layering
of APIs, where the driver is reaching behind the TTM API and calling this extra function?
Another idea is to transform the 'struct dma_addr *dma_addr' to a 'void *override_p' in
the 'struct ttm_tt'. That means still keeping the TTM API layers seperate, and "passing"
the array of DMA address through the 'override_p' array (which would be allocated by TTM DMA
code). Something along these lines (not tested):
I like this more, but I haven't actually tested it so not sure if it works right?
diff --git a/drivers/gpu/drm/nouveau/nouveau_sgdma.c b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
index e0d4474..8760a04 100644
--- a/drivers/gpu/drm/nouveau/nouveau_sgdma.c
+++ b/drivers/gpu/drm/nouveau/nouveau_sgdma.c
@@ -23,7 +23,7 @@ struct nouveau_sgdma_be {
static int
nouveau_sgdma_populate(struct ttm_backend *be, unsigned long num_pages,
struct page **pages, struct page *dummy_read_page,
- dma_addr_t *dma_addrs)
+ void *override_p)
{
struct nouveau_sgdma_be *nvbe = (struct nouveau_sgdma_be *)be;
struct drm_device *dev = nvbe->dev;
@@ -43,9 +43,9 @@ nouveau_sgdma_populate(struct ttm_backend *be, unsigned long num_pages,
nvbe->nr_pages = 0;
while (num_pages--) {
- if (dma_addrs[nvbe->nr_pages] != 0) {
- nvbe->pages[nvbe->nr_pages] =
- dma_addrs[nvbe->nr_pages];
+ dma_addr_t *ttm_dma = (dma_addr_t *)override_p;
+ if (ttm_dma && ttm_dma[nvbe->nr_pages] != 0) {
+ nvbe->pages[nvbe->nr_pages] = ttm_dma[nvbe->nr_pages];
nvbe->ttm_alloced[nvbe->nr_pages] = true;
} else {
nvbe->pages[nvbe->nr_pages] =
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 068ba09..dc700f4 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -181,7 +181,7 @@ int radeon_gart_bind(struct radeon_device *rdev, unsigned offset,
p = t / (PAGE_SIZE / RADEON_GPU_PAGE_SIZE);
for (i = 0; i < pages; i++, p++) {
- if (dma_addr[i] != 0) {
+ if (dma_addr && dma_addr[i] != 0) {
rdev->gart.ttm_alloced[p] = true;
rdev->gart.pages_addr[p] = dma_addr[i];
} else {
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 2e7419f..690545d 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -661,7 +661,7 @@ struct radeon_ttm_backend {
unsigned long num_pages;
struct page **pages;
struct page *dummy_read_page;
- dma_addr_t *dma_addrs;
+ dma_addr_t *dma_addrs; /* Can be NULL */
bool populated;
bool bound;
unsigned offset;
@@ -671,13 +671,13 @@ static int radeon_ttm_backend_populate(struct ttm_backend *backend,
unsigned long num_pages,
struct page **pages,
struct page *dummy_read_page,
- dma_addr_t *dma_addrs)
+ void *override_p)
{
struct radeon_ttm_backend *gtt;
gtt = container_of(backend, struct radeon_ttm_backend, backend);
gtt->pages = pages;
- gtt->dma_addrs = dma_addrs;
+ gtt->dma_addrs = (dma_addr_t *)override_p;
gtt->num_pages = num_pages;
gtt->dummy_read_page = dummy_read_page;
gtt->populated = true;
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index 360afb3..458727a 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -662,8 +662,7 @@ out:
/* Put all pages in pages list to correct pool to wait for reuse */
static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
- int flags, enum ttm_caching_state cstate,
- dma_addr_t *dma_address)
+ int flags, enum ttm_caching_state cstate)
{
unsigned long irq_flags;
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
@@ -707,8 +706,7 @@ static void __ttm_put_pages(struct list_head *pages, unsigned page_count,
* cached pages.
*/
static int __ttm_get_pages(struct list_head *pages, int flags,
- enum ttm_caching_state cstate, unsigned count,
- dma_addr_t *dma_address)
+ enum ttm_caching_state cstate, unsigned count)
{
struct ttm_page_pool *pool = ttm_get_pool(flags, cstate);
struct page *p = NULL;
@@ -766,7 +764,7 @@ static int __ttm_get_pages(struct list_head *pages, int flags,
printk(KERN_ERR TTM_PFX
"Failed to allocate extra pages "
"for large request.");
- __ttm_put_pages(pages, 0, flags, cstate, NULL);
+ __ttm_put_pages(pages, 0, flags, cstate);
return r;
}
}
@@ -859,19 +857,19 @@ int ttm_page_alloc_debugfs(struct seq_file *m, void *data)
}
EXPORT_SYMBOL(ttm_page_alloc_debugfs);
int ttm_get_pages(struct ttm_tt *ttm, struct list_head *pages,
- unsigned count, dma_addr_t *dma_address)
+ unsigned count, int index)
{
if (ttm->be && ttm->be->func && ttm->be->func->get_pages)
- return ttm->be->func->get_pages(ttm, pages, count, dma_address);
+ return ttm->be->func->get_pages(ttm, pages, count, index);
return __ttm_get_pages(pages, ttm->page_flags, ttm->caching_state,
- count, dma_address);
+ count);
}
void ttm_put_pages(struct ttm_tt *ttm, struct list_head *pages,
- unsigned page_count, dma_addr_t *dma_address)
+ unsigned page_count, int index)
{
if (ttm->be && ttm->be->func && ttm->be->func->put_pages)
- ttm->be->func->put_pages(ttm, pages, page_count, dma_address);
+ ttm->be->func->put_pages(ttm, pages, page_count, index);
else
__ttm_put_pages(pages, page_count, ttm->page_flags,
- ttm->caching_state, dma_address);
+ ttm->caching_state);
}
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
index a5be62e..08e182f 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc_dma.c
@@ -45,6 +45,7 @@
#include <linux/atomic.h>
#include <linux/device.h>
#include <linux/kthread.h>
+#include <drm/drm_mem_util.h>
#include "ttm/ttm_bo_driver.h"
#include "ttm/ttm_page_alloc.h"
#ifdef TTM_HAS_AGP
@@ -1097,7 +1098,7 @@ out:
* cached pages. On failure will hold the negative return value (-ENOMEM, etc).
*/
int ttm_dma_get_pages(struct ttm_tt *ttm, struct list_head *pages,
- unsigned count, dma_addr_t *dma_address)
+ unsigned count, int idx)
{
int r = -ENOMEM;
@@ -1105,6 +1106,11 @@ int ttm_dma_get_pages(struct ttm_tt *ttm, struct list_head *pages,
gfp_t gfp_flags;
enum pool_type type;
struct device *dev = ttm->be->dev;
+ dma_addr_t *dma_address;
+
+ /* We _MUST_ have a proper index value. */
+ if (WARN_ON(idx < 0));
+ return -EINVAL;
type = ttm_to_type(ttm->page_flags, ttm->caching_state);
@@ -1129,6 +1135,7 @@ int ttm_dma_get_pages(struct ttm_tt *ttm, struct list_head *pages,
cstate);
}
#endif
+ dma_address = &((dma_addr_t *)ttm->override_p)[idx];
/* Take pages out of a pool (if applicable) */
r = ttm_dma_pool_get_pages(pool, pages, dma_address, count);
/* clear the pages coming from the pool if requested */
@@ -1201,7 +1208,7 @@ static int ttm_dma_pool_get_num_unused_pages(void)
/* Put all pages in pages list to correct pool to wait for reuse */
void ttm_dma_put_pages(struct ttm_tt *ttm, struct list_head *pages,
- unsigned page_count, dma_addr_t *dma_address)
+ unsigned page_count, int idx)
{
struct dma_pool *pool;
enum pool_type type;
@@ -1230,9 +1237,12 @@ void ttm_dma_put_pages(struct ttm_tt *ttm, struct list_head *pages,
count = ttm_dma_put_pages_in_pool(pool, pages, page_count, is_cached);
- for (i = 0; i < count; i++)
- dma_address[i] = 0;
-
+ /* Optional. */
+ if (idx >= 0) {
+ dma_addr_t *dma_address = &((dma_addr_t *)ttm->override_p)[idx];
+ for (i = 0; i < count; i++)
+ dma_address[i] = 0;
+ }
spin_lock_irqsave(&pool->lock, irq_flags);
pool->npages_in_use -= count;
if (is_cached)
@@ -1386,11 +1396,27 @@ int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data)
return 0;
}
EXPORT_SYMBOL_GPL(ttm_dma_page_alloc_debugfs);
+
+static int ttm_dma_alloc_priv(struct ttm_tt *ttm)
+{
+ ttm->override_p = drm_calloc_large(ttm->num_pages, sizeof(dma_addr_t *));
+ if (WARN_ON(!ttm->override_p))
+ return -ENOMEM;
+ return 0;
+
+}
+static void ttm_dma_free_priv(struct ttm_tt *ttm)
+{
+ drm_free_large(ttm->override_p);
+ ttm->override_p = NULL;
+}
bool ttm_dma_override(struct ttm_backend_func *be)
{
if (swiotlb_nr_tbl() && be && !ttm_dma_disable) {
be->get_pages = &ttm_dma_get_pages;
be->put_pages = &ttm_dma_put_pages;
+ be->alloc_priv = &ttm_dma_alloc_priv;
+ be->free_priv = &ttm_dma_free_priv;
return true;
}
return false;
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index 31ae359..0f0d57f 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -50,16 +50,16 @@ static int ttm_tt_swapin(struct ttm_tt *ttm);
static void ttm_tt_alloc_page_directory(struct ttm_tt *ttm)
{
ttm->pages = drm_calloc_large(ttm->num_pages, sizeof(*ttm->pages));
- ttm->dma_address = drm_calloc_large(ttm->num_pages,
- sizeof(*ttm->dma_address));
+ if (ttm->be && ttm->be->func && ttm->be->func->alloc_priv)
+ ttm->be->func->alloc_priv(ttm);
}
static void ttm_tt_free_page_directory(struct ttm_tt *ttm)
{
drm_free_large(ttm->pages);
ttm->pages = NULL;
- drm_free_large(ttm->dma_address);
- ttm->dma_address = NULL;
+ if (ttm->be && ttm->be->func && ttm->be->func->free_priv)
+ ttm->be->func->free_priv(ttm);
}
static void ttm_tt_free_user_pages(struct ttm_tt *ttm)
@@ -110,7 +110,7 @@ static struct page *__ttm_tt_get_page(struct ttm_tt *ttm, int index)
INIT_LIST_HEAD(&h);
- ret = ttm_get_pages(ttm, &h, 1, &ttm->dma_address[index]);
+ ret = ttm_get_pages(ttm, &h, 1, index);
if (ret != 0)
return NULL;
@@ -169,7 +169,7 @@ int ttm_tt_populate(struct ttm_tt *ttm)
}
be->func->populate(be, ttm->num_pages, ttm->pages,
- ttm->dummy_read_page, ttm->dma_address);
+ ttm->dummy_read_page, ttm->override_p);
ttm->state = tt_unbound;
return 0;
}
@@ -303,7 +303,7 @@ static void ttm_tt_free_alloced_pages(struct ttm_tt *ttm, bool call_clear)
count++;
}
}
- ttm_put_pages(ttm, &h, count, ttm->dma_address);
+ ttm_put_pages(ttm, &h, count, 0 /* start at zero and go up to count */);
ttm->state = tt_unpopulated;
ttm->first_himem_page = ttm->num_pages;
ttm->last_lomem_page = -1;
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 1826c3b..771697a 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -58,7 +58,7 @@ struct ttm_backend_func {
int (*populate) (struct ttm_backend *backend,
unsigned long num_pages, struct page **pages,
struct page *dummy_read_page,
- dma_addr_t *dma_addrs);
+ void *override_p);
/**
* struct ttm_backend_func member clear
*
@@ -109,10 +109,11 @@ struct ttm_backend_func {
*
* @ttm: ttm which contains flags for page allocation and caching state.
* @pages: head of empty linked list where pages are filled.
- * @dma_address: The DMA (bus) address of pages
+ * @idx: The current index in ttm->pages[] array. Negative means
+ * don't assume ttm->pages[idx] order matches the order in *pages.
*/
int (*get_pages) (struct ttm_tt *ttm, struct list_head *pages,
- unsigned count, dma_addr_t *dma_address);
+ unsigned count, int idx);
/**
* ttm_put_pages override. The backend can override the default
@@ -124,10 +125,17 @@ struct ttm_backend_func {
* @pages: list of pages to free.
* @page_count: number of pages in the list. Zero can be passed for
* unknown count.
- * @dma_address: The DMA (bus) address of pages
+ * @idx: The current index in the ttm->pages[] array. Negative means
+ * don't assume ttm->pages[idx] order matches the order in *pages.
*/
void (*put_pages) (struct ttm_tt *ttm, struct list_head *pages,
- unsigned page_count, dma_addr_t *dma_address);
+ unsigned page_count, int idx);
+
+ /**
+ * TODO: Flesh this out.
+ */
+ int (*alloc_priv) (struct ttm_tt *ttm);
+ void (*free_priv) (struct ttm_tt *ttm);
};
/**
@@ -207,7 +215,7 @@ struct ttm_tt {
tt_unbound,
tt_unpopulated,
} state;
- dma_addr_t *dma_address;
+ void *override_p;
};
#define TTM_MEMTYPE_FLAG_FIXED (1 << 0) /* Fixed (on-card) PCI memory */
diff --git a/include/drm/ttm/ttm_page_alloc.h b/include/drm/ttm/ttm_page_alloc.h
index daf5db6..31e6079 100644
--- a/include/drm/ttm/ttm_page_alloc.h
+++ b/include/drm/ttm/ttm_page_alloc.h
@@ -68,12 +68,12 @@ static inline int ttm_dma_page_alloc_debugfs(struct seq_file *m, void *data)
* @ttm: ttm which contains flags for page allocation and caching state.
* @pages: heado of empty linked list where pages are filled.
* @count: number of pages to allocate.
- * @dma_address: The DMA (bus) address of pages - (by default zero).
+ * @idx: The current index in ttm->pages[idx]. Negative means ignore.
*/
int ttm_get_pages(struct ttm_tt *ttm,
struct list_head *pages,
unsigned count,
- dma_addr_t *dma_address);
+ int idx);
/**
* Put linked list of pages to pool.
*
@@ -81,12 +81,12 @@ int ttm_get_pages(struct ttm_tt *ttm,
* @pages: list of pages to free.
* @page_count: number of pages in the list. Zero can be passed for unknown
* count.
- * @dma_address: The DMA (bus) address of pages (by default zero).
+ * @idx: The current index in ttm->pages[idx]. Negative means ignore.
*/
void ttm_put_pages(struct ttm_tt *ttm,
struct list_head *pages,
unsigned page_count,
- dma_addr_t *dma_address);
+ int idx);
/**
* Initialize pool allocator.
*/
^ permalink raw reply related
* Re: [Qemu-devel] [Qemu-ppc] [PATCH] pseries: Correct vmx/dfp handling in both KVM and TCG cases
From: Alexander Graf @ 2011-10-24 17:25 UTC (permalink / raw)
To: David Gibson; +Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org
In-Reply-To: <20111024052951.GE4157@truffala.fritz.box>
On 23.10.2011, at 22:29, David Gibson wrote:
> On Thu, Oct 20, 2011 at 11:49:40PM -0700, Alexander Graf wrote:
>>
>> On 20.10.2011, at 22:06, David Gibson wrote:
>>
>>> On Thu, Oct 20, 2011 at 07:40:00PM -0700, Alexander Graf wrote:
>>>> On 20.10.2011, at 17:41, David Gibson <david@gibson.dropbear.id.au> wrote:
>>>>> On Thu, Oct 20, 2011 at 10:12:51AM -0700, Alexander Graf wrote:
>>>>>> On 17.10.2011, at 21:15, David Gibson wrote:
>>> [snip]
>>>>> So, I really don't follow what the logic you want is. It sounds more
>>>>> like what I have already, so I'm not sure how -cpu host comes into
>>>>> this.
>>>>
>>>> Well, I want something very simple, layered:
>>>>
>>>> -cpu host only searches for pvr matches and selects a different CPU
>>>> -type based on this
>>>
>>> Hrm, ok, well I can do this if you like, but note that this is quite
>>> different from how -cpu host behaves on x86. There it builds the CPU
>>> spec from scratch based on querying the host cpuid, rather than
>>> selecting from an existing list of cpus. I selected from the existing
>>> table based on host PVR because that was the easiest source for some
>>> of the info in the cpu_spec, but my intention was that anything we
>>> _can_ query directly from the host would override the table.
>>>
>>> It seems to be your approach is giving up on the possibility of
>>> allowing -cpu host to work (and give you full access to the host
>>> features) when qemu doesn't recognize the precise PVR of the host cpu.
>>
>> I disagree :). This is what x86 does:
>>
>> * -cpu host fetches CPUID info from host, puts it into vcpu
>> * vcpu CPUID info gets ANDed with KVM capability CPUIDs
>>
>> I want basically the same thing. I want to have 2 different layers
>> for 2 different semantics. One for what the host CPU would be able
>> to do and one for what we can emulate, and two different steps to
>> ensure control over them.
>>
>> The thing I think I'm apparently not bringing over yet is that I'm
>> more than happy to get rid of the PVR searching step for -cpu host
>> and instead use a full host capability inquiry mechanism. But that
>> inquiry should indicate what the host CPU can do. It has nothing to
>> do with KVM yet. The masking with KVM capabilities should be the
>> next separate step.
>>
>> My goal is really to separate different layers into actual different
>> layers :).
>
> Hrm. I think I see what you're getting at. Although nothing in that
> patch is about kvm capabilities - it's all about working out what the
> host's cpu can do.
Reading through the patch again I think I see your point now :). Yes, the kvmppc_host_cpu_def function only tries to fetch the host CPU capabilities.
So yes, there is basically only the masking part with what we can actually virtualize missing. But for now we can just assume that every feature the host CPU supports is available.
I'll apply your patch for now, as it certainly is better than what we had before.
>
>>> This gets further complicated in the case of the w-i-p patch I have to
>>> properly advertise page sizes, where it's not just presence or absence
>>> of a feature, but the specific SLB and HPTE encodings must be
>>> advertised to the guest.
>>
>> Yup, so we'd read out the host dt to find the host possible
>> encodings (probably a bad idea, but that's a different story)
>
> Um, a different story perhaps, but one I kind of need an answer to in
> the near future... I can query the host cpu's page sizes easily
> enough, but I'm really not sure where this should be stashed before
> filtering as suggested below.
Page sizes are usually powers of 2, so we should be ok with just having a bitmap there with each bit meaning 1 << (n + 12).
Alex
^ permalink raw reply
* Re: [patch net-next V4] net: introduce ethernet teaming device
From: Michał Mirosław @ 2011-10-24 17:22 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, eric.dumazet, bhutchings, shemminger, fubar, andy,
tgraf, ebiederm, kaber, greearb, jesse, fbl, benjamin.poirier,
jzupka
In-Reply-To: <1319444005-1281-1-git-send-email-jpirko@redhat.com>
2011/10/24 Jiri Pirko <jpirko@redhat.com>:
> This patch introduces new network device called team. It supposes to be
> very fast, simple, userspace-driven alternative to existing bonding
> driver.
[...]
> drivers/net/team/team.c | 1573 +++++++++++++++++++++++++++++
> drivers/net/team/team_mode_activebackup.c | 152 +++
> drivers/net/team/team_mode_roundrobin.c | 107 ++
I think this mode-modularity is overkill. One mode will compile to at
most a few hundred bytes of code+data, but will use at least 10 times
that to get loaded and tracked properly. How often/how many more modes
you anticipate to be introduced? You could just keep the modular
design but drop the kernel module separation and maybe have modes
conditionally compiled (for those from the embedded world squeezing
every byte).
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [RFC][PATCH] KVM: Introduce direct MSI message injection for in-kernel irqchips
From: Michael S. Tsirkin @ 2011-10-24 17:23 UTC (permalink / raw)
To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm
In-Reply-To: <20111024170508.GB30385@redhat.com>
On Mon, Oct 24, 2011 at 07:05:08PM +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 24, 2011 at 06:10:28PM +0200, Jan Kiszka wrote:
> > On 2011-10-24 18:05, Michael S. Tsirkin wrote:
> > >> This is what I have in mind:
> > >> - devices set PBA bit if MSI message cannot be sent due to mask (*)
> > >> - core checks&clears PBA bit on unmask, injects message if bit was set
> > >> - devices clear PBA bit if message reason is resolved before unmask (*)
> > >
> > > OK, but practically, when exactly does the device clear PBA?
> >
> > Consider a network adapter that signals messages in a RX ring: If the
> > corresponding vector is masked while the guest empties the ring, I
> > strongly assume that the device is supposed to take back the pending bit
> > in that case so that there is no interrupt inject on a later vector
> > unmask operation.
> >
> > Jan
>
> Do you mean virtio here? Do you expect this optimization to give
> a significant performance gain?
It would also be challenging to implement this in
a race free manner. Clearing on interrupt status read
seems straight-forward.
> > --
> > Siemens AG, Corporate Technology, CT T DE IT 1
> > Corporate Competence Center Embedded Linux
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.