Re: [Dri-devel] MACH64_BM_GUI_TABLE(_CMD)?
On 2002.05.05 19:41 Frank C. Earl wrote:
> ...
>
> > I plan to build a test case for this, but I would like to hear
> preliminary
> > opinions about this, in case I'm missing something. Frank, have you
> tested
> > this before?
>
> Yes, pretty extensively, but I didn't have time to set up tests for
> spanning
> multiple pages- we ought to do that one last one before commiting to the
> path we're now looking at.
Ok. I've made a test to see if this is possible and it failed. It's best
that Leif and Frank made a quick review of the test I made (attached), to
see if is there any mistake I made, before we put a stone on this issue.
Basically I changed mach64_bm_dma_test to allocate a 2nd descriptor table
and 2 more data buffers. The first buffer attempts to override
MACH64_BM_GUI_TABLE to read the 2nd table (which points to a 3rd buffer
which fille the vertex registers with different values). The 2nd buffer is
the continuation of the 1st and has the regular cleanup.
Now I plan to reproduce the hang that we had when trying to draw a
multitextured triangle without the texture offset specified to see if the
engine can recover or not from the lock. Frank, on IRC I got the
impression that you were gonna try this. Did you?
José Fonseca
static int mach64_bm_dma_test( drm_device_t *dev )
{
drm_mach64_private_t *dev_priv = dev->dev_private;
dma_addr_t data_handle, data2_handle, data3_handle, table2_handle;
void *cpu_addr_data, *cpu_addr_data2, *cpu_addr_data3, *cpu_addr_table2;
u32 data_addr, data2_addr, data3_addr, table2_addr;
u32 *table, *data, *table2, *data2, *data3;
u32 regs[3], expected[3];
int i;
DRM_DEBUG( "%s\n", __FUNCTION__ );
table = (u32 *) dev_priv->cpu_addr_table;
/* FIXME: get a dma buffer from the freelist here rather than using the pool */
DRM_DEBUG( "Allocating data memory ...\n" );
cpu_addr_data = pci_pool_alloc( dev_priv->pool, SLAB_ATOMIC, &data_handle );
cpu_addr_data2 = pci_pool_alloc( dev_priv->pool, SLAB_ATOMIC, &data2_handle );
cpu_addr_data3 = pci_pool_alloc( dev_priv->pool, SLAB_ATOMIC, &data3_handle );
cpu_addr_table2 = pci_pool_alloc( dev_priv->pool, SLAB_ATOMIC, &table2_handle
);
if (!cpu_addr_data || !data_handle || !cpu_addr_data2 || !data2_handle ||
!cpu_addr_data3 || !data3_handle || !cpu_addr_table2 || !table2_handle) {
DRM_INFO( "data-memory allocation failed!\n" );
return -ENOMEM;
} else {
data = (u32 *) cpu_addr_data;
data_addr = (u32) data_handle;
data2 = (u32 *) cpu_addr_data2;
data2_addr = (u32) data2_handle;
data3 = (u32 *) cpu_addr_data3;
data3_addr = (u32) data3_handle;
table2 = (u32 *) cpu_addr_table2;
table2_addr = (u32) table2_handle;
}
MACH64_WRITE( MACH64_SRC_CNTL, 0x );
MACH64_WRITE( MACH64_VERTEX_1_S, 0x );
MACH64_WRITE( MACH64_VERTEX_1_T, 0x );
MACH64_WRITE( MACH64_VERTEX_1_W, 0x );
for (i=0; i < 3; i++) {
DRM_DEBUG( "(Before DMA Transfer) reg %d = 0x%08x\n", i,
MACH64_READ( (MACH64_VERTEX_1_S + i*4) ) );
}
/* 1_90 = VERTEX_1_S, setup 3 sequential reg writes */
/* use only s,t,w vertex registers so we don't have to mask any results */
data[0] = cpu_to_le32(0x00020190);
data[1] = expected[0] = 0x;
data[2] = expected[1] = 0x;
data[3] = expected[2] = 0x;
data[4] = cpu_to_le32(MACH64_BM_GUI_TABLE_CMD);
data[5] = cpu_to_le32(table2_addr | MACH64_CIRCULAR_BUF_SIZE_16KB);
data[6] = cpu_to_le32(MACH64_DST_HEIGHT_WIDTH);
data[7] = cpu_to_le32(0);
data2[8] = cpu_to_le32(0x006d); /* SRC_CNTL */
data2[9] = 0x;
data3[0] = cpu_to_le32(0x00020190);
data3[1] = 0x;
data3[2] = 0x;
data3[3] = 0x;
data3[4] = cpu_to_le32(0x006d); /* SRC_CNTL */
data3[5] = 0x;
DRM_DEBUG( "Preparing table ...\n" );
table[0] = cpu_to_le32(MACH64_BM_ADDR + APERTURE_OFFSET);
table[1] = cpu_to_le32(data_addr);
table[2] = cpu_to_le32(8 * sizeof( u32 ) | 0x4000);
table[3] = 0;
table[4] = cpu_to_le32(MACH64_BM_ADDR + APERTURE_OFFSET);
table[5] = cpu_to_le32(data2_addr);
table[6] = cpu_to_le32(2 * sizeof( u32 ) | 0x8000 | 0x4000);
table[7] = 0;
table2[0] = cpu_to_le32(MACH64_BM_ADDR + APERTURE_OFFSET);
table2[1] = cpu_to_le32(data3_addr);
table2[2] = cpu_to_le32(6 * sizeof( u32 ) | 0x8000 | 0x4000);
table2[3] = 0;
DRM_DEBUG( "table[0] = 0x%08x\n", table[0] );
DRM_DEBUG( "table[1] = 0x%08x\n", table[1] );
Re: [Dri-devel] MACH64_BM_GUI_TABLE(_CMD)?
On Friday 03 May 2002 09:49 am, you wrote: > As I was studying the specs and code to be able to understand and reply to > Leif's previous post (which I haven't completed yet..), I noticed at the > same time a bug and a feature which could mean that blind client buffering > could be insecure after all. After testing, it doesn't appear to be. > Only commands which are on block 0 of MMIO region can be streamed into a > GUI master operation, as said in the BM_DATA register spec (8-11). The > MACH64_BM_GUI_TABLE_CMD is an alias in this block exactly for this > purpose, i.e., to be streamed trhough the GUI command FIFO, as said in its > spec (8-12). Doesn't this means that we can initiate further GUI master > operations from a command buffer since, once the first GUI master > operation is setup, it's only necessary to set MACH64_BM_GUI_TABLE_CMD and > MACH64_DST_HEIGHT_WIDTH to fire it up - both accessible from GUI FIFO. Since, upon examination, many of the pieces of documentation we have gotten for the display chips are either incomplete, testing a theory is the best way of knowing whether or not something will do what you expect. I thought that it might be that way, so I tested to see if I could I could re-submit new GUI-master operations to the engine. It appears from the tests that I ran that the GUI-master operation only works with GUI setup and GUI comand registers. Setting up a new GUI-master or BLIT operation did nothing and seemed to just leave the registers set with those values (this was for one descriptor entry's worth of data- we may need to set up a test with MUCH larger spaces with the code in place to be certain...) no actions were carried out (probably because they're not FIFOed and the engine WAS busy...). > Although firing up a stream of arbitrary commands shouldn't be a reason > for concern since the commands are only innocent(?) GUI operations, this > gives the ability of setting up any descriptor table. One consequence is > that if this table is invalid the whole DMA engine is unnoperational until > a cold reset. Another is that they can access to any register... Yes, and they can set it to any value, but the engine doesn't seem to act on any command settings done this way. It is one of the reasons why I said we ought to push a few register reset operations on some of the critical registers (BUS_CNTL, for example) so that if something DOES initiate a command immedately afterward that expects the settings that were there prior to a DMA pass they won't be broken. You can set up a new descriptor table, but there is no direct way to fire it off after the pass is over with and you can't do it in the middle of a pass (Because your GUI commands needed to fire off a pass are NOT put in the FIFO, they're executed directly from the bus-mastering engine...)- so, if you reset the bus-master control registers at the end of a pass, there will be nothing a malicious client could do to initiate their own passes, bogus or otherwise. > I plan to build a test case for this, but I would like to hear preliminary > opinions about this, in case I'm missing something. Frank, have you tested > this before? Yes, pretty extensively, but I didn't have time to set up tests for spanning multiple pages- we ought to do that one last one before commiting to the path we're now looking at. -- Frank Earl ___ Have big pipes? SourceForge.net is looking for download mirrors. We supply the hardware. You get the recognition. Email Us: [EMAIL PROTECTED] ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] MACH64_BM_GUI_TABLE(_CMD)?
On Friday 03 May 2002 11:08 am, you wrote: > I think the reason for the alias is that the card increments the > GUI_TABLE_ADDR @ BM_GUI_TABLE as it consumes descriptors, so writing to > BM_GUI_TABLE could disrupt a DMA pass in progress. Using the alias > ensures that the commands already in the FIFO/command buffers are > processed before changing the table address. However, as you say below, > it could potentially be used inside a command buffer as well. I tested that in my tests as well. What appears to happen is as follows: The alias is queued up in the FIFO and is synced against the stream. The other is immediately executed and if you're busy, changing it doesn't seem to do anything other than change it after the fact. Attempts to alter the head of the descriptor table or my position in the table didn't do anything and the descriptor table that was specified seemed to execute correctly. Now, for longer chains, it may be that it is a problem- I only tested for one 4K page of data. > > Only commands which are on block 0 of MMIO region can be streamed into a > > GUI master operation, as said in the BM_DATA register spec (8-11). > > This can't be right since the setup engine registers are in block 1 and we > are using them in a GUI master op. That's what BUS_EXT_REG_EN @ BUS_CNTL > is for. I think that it's more of something like the engine only works with GUI engine commands, of which most of them are in Block 0. I've found that the documentation's a good starting point, but it's not consistent or completely accurate. > We definitely don't want clients changing the descriptor table head from > inside a dma buffer. It currently appears that the chip won't let you do that, thankfully. > If it's really possible to do this, you might be able to set up a blit > as well. This doesn't constitute a GUI operation, and it doesn't seem to let you do anything other than issue stuff to the GUI registers with any effect. I tried every one of the possible stunts to get the engine to do something untoward and couldn't get it to do anything other than rendering commands. A BLIT, while we think of it as a GUI operation, isn't one as far as the chip is concerned, it's a DMA operation- which doesn't get acted upon until the current pass is completed. > btw, I also noticed that HOST_CNTL has a bit for big-endian translation of > host data. At 15 and 16bpp, it swaps bytes within each word and at 32bpp > it reverses the order of the 4 bytes within each dword. This might fix > Peter's texture problem. I do believe that might be what that is there for... > Yeah, the first thing is to determine if it's really possible to kick off > a second DMA pass from within a DMA buffer. Already done, but if you want to reverify (just in case I may have missed something, i.e. since I didn't test past one descriptor entry, it might be that under some cases there's a loophole that allows someone to arbitrarily fire off their own DMA operations...)- go right on ahead. I want this to be the best performing solution for the chip, but also the most secure possible. -- Frank Earl ___ Have big pipes? SourceForge.net is looking for download mirrors. We supply the hardware. You get the recognition. Email Us: [EMAIL PROTECTED] ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] MACH64_BM_GUI_TABLE(_CMD)?
On Fri, 2002-05-03 at 18:08, Leif Delgass wrote: > On Fri, 3 May 2002, José Fonseca wrote: > > btw, I also noticed that HOST_CNTL has a bit for big-endian translation of > host data. At 15 and 16bpp, it swaps bytes within each word and at 32bpp > it reverses the order of the 4 bytes within each dword. This might fix > Peter's texture problem. Yes, if it allows to freely choose the three ways to swap. That's how I'm going to solve this in the radeon driver (and probably could have in r128 if I had noticed this then :). -- Earthling Michel Dänzer (MrCooper)/ Debian GNU/Linux (powerpc) developer XFree86 and DRI project member / CS student, Free Software enthusiast ___ Have big pipes? SourceForge.net is looking for download mirrors. We supply the hardware. You get the recognition. Email Us: [EMAIL PROTECTED] ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] MACH64_BM_GUI_TABLE(_CMD)?
On 2002.05.03 17:08 Leif Delgass wrote: > On Fri, 3 May 2002, José Fonseca wrote: > > > As I was studying the specs and code to be able to understand and reply > to > > Leif's previous post (which I haven't completed yet..), I noticed at > the > > same time a bug and a feature which could mean that blind client > buffering > > could be insecure after all. > > > > The bug is that we should be using MACH64_BM_GUI_TABLE and not > > MACH64_BM_GUI_TABLE_CMD when setting up the GUI master operation. The > > difference between the two is that the later is queued in the FIFO and > the > > former not, and we really don't want this as it could get in the way > later. > > I think the reason for the alias is that the card increments the > GUI_TABLE_ADDR @ BM_GUI_TABLE as it consumes descriptors, so writing to > BM_GUI_TABLE could disrupt a DMA pass in progress. Using the alias > ensures that the commands already in the FIFO/command buffers are > processed before changing the table address. So you think that it's less prone to error use the BM_GUI_TABLE_CMD alias to setup the GUI master operation. > However, as you say below, > it could potentially be used inside a command buffer as well. > > > Only commands which are on block 0 of MMIO region can be streamed into > a > > GUI master operation, as said in the BM_DATA register spec (8-11). > > This can't be right since the setup engine registers are in block 1 and > we > are using them in a GUI master op. That's what BUS_EXT_REG_EN @ BUS_CNTL > is for. Good point. Although it's not documented anywhere that BUS_EXT_REG_EN @ BUS_CNTL has any relation with the GUI master operation (especially because during a GUI master operation the registers are not written trhough the apperture but internally through BM_ADDR/BM_DATA). > ... > > btw, I also noticed that HOST_CNTL has a bit for big-endian translation > of > host data. At 15 and 16bpp, it swaps bytes within each word and at 32bpp > it reverses the order of the 4 bytes within each dword. This might fix > Peter's texture problem. It's still not clear that this will solve Peter's problem, because, assuming the textures are 16bit, the words are swaped, but _not_ the bytes. The question is where did the bytes got swapped? I think Mesa probably stores pixels in little endian format, or doesn't even try to change the pixel format. If so, how did the other drivers addressed this problem. (Note these are retorical question as I just didn't had the time to look the answers in the code - I'm still a little more concerned with the MMIO on PowerPC architecture). > ... > > Yeah, the first thing is to determine if it's really possible to kick off > a second DMA pass from within a DMA buffer. > I'll report when I get some results. José Fonseca ___ Have big pipes? SourceForge.net is looking for download mirrors. We supply the hardware. You get the recognition. Email Us: [EMAIL PROTECTED] ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] MACH64_BM_GUI_TABLE(_CMD)?
On Fri, 3 May 2002, José Fonseca wrote: > As I was studying the specs and code to be able to understand and reply to > Leif's previous post (which I haven't completed yet..), I noticed at the > same time a bug and a feature which could mean that blind client buffering > could be insecure after all. > > The bug is that we should be using MACH64_BM_GUI_TABLE and not > MACH64_BM_GUI_TABLE_CMD when setting up the GUI master operation. The > difference between the two is that the later is queued in the FIFO and the > former not, and we really don't want this as it could get in the way later. I think the reason for the alias is that the card increments the GUI_TABLE_ADDR @ BM_GUI_TABLE as it consumes descriptors, so writing to BM_GUI_TABLE could disrupt a DMA pass in progress. Using the alias ensures that the commands already in the FIFO/command buffers are processed before changing the table address. However, as you say below, it could potentially be used inside a command buffer as well. > Only commands which are on block 0 of MMIO region can be streamed into a > GUI master operation, as said in the BM_DATA register spec (8-11). This can't be right since the setup engine registers are in block 1 and we are using them in a GUI master op. That's what BUS_EXT_REG_EN @ BUS_CNTL is for. > The MACH64_BM_GUI_TABLE_CMD is an alias in this block exactly for this > purpose, i.e., to be streamed trhough the GUI command FIFO, as said in > its spec (8-12). Doesn't this means that we can initiate further GUI > master operations from a command buffer since, once the first GUI master > operation is setup, it's only necessary to set MACH64_BM_GUI_TABLE_CMD > and MACH64_DST_HEIGHT_WIDTH to fire it up - both accessible from GUI > FIFO. We definitely don't want clients changing the descriptor table head from inside a dma buffer. > Although firing up a stream of arbitrary commands shouldn't be a reason > for concern since the commands are only innocent(?) GUI operations, this > gives the ability of setting up any descriptor table. One consequence is > that if this table is invalid the whole DMA engine is unnoperational until > a cold reset. Another is that they can access to any register... If it's really possible to do this, you might be able to set up a blit as well. btw, I also noticed that HOST_CNTL has a bit for big-endian translation of host data. At 15 and 16bpp, it swaps bytes within each word and at 32bpp it reverses the order of the 4 bytes within each dword. This might fix Peter's texture problem. > I plan to build a test case for this, but I would like to hear preliminary > opinions about this, in case I'm missing something. Frank, have you tested > this before? Yeah, the first thing is to determine if it's really possible to kick off a second DMA pass from within a DMA buffer. -- Leif Delgass http://www.retinalburn.net ___ Have big pipes? SourceForge.net is looking for download mirrors. We supply the hardware. You get the recognition. Email Us: [EMAIL PROTECTED] ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
