qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] CPUTLBEntry Question
@ 2007-06-13 20:25 Ryan Riley
  2007-06-14 13:22 ` Paul Brook
  2007-06-14 13:41 ` amateur
  0 siblings, 2 replies; 6+ messages in thread
From: Ryan Riley @ 2007-06-13 20:25 UTC (permalink / raw)
  To: qemu-devel

I'm making some small changes to the TLB stuff in QEMU 0.9.0
(specifically, I'm only working with i386-softmmu) and have run into
an odd question I'm hoping someone can answer for me.  The CPUTLBEntry
structure definition in cpu-defs.h looks like this...

typedef struct CPUTLBEntry {
    /* bit 31 to TARGET_PAGE_BITS : virtual address
       bit TARGET_PAGE_BITS-1..IO_MEM_SHIFT : if non zero, memory io
                                              zone number
       bit 3                      : indicates that the entry is invalid
       bit 2..0                   : zero
    */
    target_ulong addr_read;
    target_ulong addr_write;
    target_ulong addr_code;
    /* addend to virtual address to get physical address */
    target_phys_addr_t addend;
} CPUTLBEntry;

If I change it to add another member, like so..

typedef struct CPUTLBEntry {
    /* bit 31 to TARGET_PAGE_BITS : virtual address
       bit TARGET_PAGE_BITS-1..IO_MEM_SHIFT : if non zero, memory io
                                              zone number
       bit 3                      : indicates that the entry is invalid
       bit 2..0                   : zero
    */
    target_ulong addr_read;
    target_ulong addr_write;
    target_ulong addr_code;
    /* addend to virtual address to get physical address */
    target_phys_addr_t addend;
    /* New member */
    target_phys_addr_t blah;
} CPUTLBEntry;

then QEMU crashes on startup.  (It also crashes if I put that blah
entry on the beginning instead of the end.)  I'm sure there's code
somewhere that must be making assumptions about the size of TLB entry,
but I'm at a loss for finding it.  (I have noticed that the assembly
code in softmmu_header.h indexes to the addend based on addr_read or
addr_write, but adding a new member to the end of the structure
shouldn't impact that, right?)

If anyone has any insight, I would be very appreciative.

Thanks
Ryan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] CPUTLBEntry Question
  2007-06-13 20:25 [Qemu-devel] CPUTLBEntry Question Ryan Riley
@ 2007-06-14 13:22 ` Paul Brook
  2007-06-14 19:31   ` Ryan Riley
  2007-06-14 13:41 ` amateur
  1 sibling, 1 reply; 6+ messages in thread
From: Paul Brook @ 2007-06-14 13:22 UTC (permalink / raw)
  To: qemu-devel; +Cc: Ryan Riley

> then QEMU crashes on startup.  (It also crashes if I put that blah
> entry on the beginning instead of the end.)  I'm sure there's code
> somewhere that must be making assumptions about the size of TLB entry,
> but I'm at a loss for finding it.  (I have noticed that the assembly
> code in softmmu_header.h indexes to the addend based on addr_read or
> addr_write, but adding a new member to the end of the structure
> shouldn't impact that, right?)

Wrong. The assembly implementation assumes CPUTLBEntry is 16 bytes (or to be 
exact 1 << CPU_TLB_ENTRY_BITS).

The C implementation in softmmu_header.h will work with any layout.

Paul

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] CPUTLBEntry Question
  2007-06-13 20:25 [Qemu-devel] CPUTLBEntry Question Ryan Riley
  2007-06-14 13:22 ` Paul Brook
@ 2007-06-14 13:41 ` amateur
  2007-06-14 14:00   ` Blue Swirl
  1 sibling, 1 reply; 6+ messages in thread
From: amateur @ 2007-06-14 13:41 UTC (permalink / raw)
  To: qemu-devel

On Wed, Jun 13, 2007 at 04:25:07PM -0400, Ryan Riley wrote:
> typedef struct CPUTLBEntry {
>    /* bit 31 to TARGET_PAGE_BITS : virtual address
>       bit TARGET_PAGE_BITS-1..IO_MEM_SHIFT : if non zero, memory io
>                                              zone number
>       bit 3                      : indicates that the entry is invalid
>       bit 2..0                   : zero
>    */
>    target_ulong addr_read;
>    target_ulong addr_write;
>    target_ulong addr_code;
>    /* addend to virtual address to get physical address */
>    target_phys_addr_t addend;
> } CPUTLBEntry;
> 
> If I change it to add another member, like so..
> 
> typedef struct CPUTLBEntry {
>    /* New member */
>    target_phys_addr_t blah;
> } CPUTLBEntry;
> 
> then QEMU crashes on startup.  (It also crashes if I put that blah
> entry on the beginning instead of the end.)  I'm sure there's code
> somewhere that must be making assumptions about the size of TLB entry,
> but I'm at a loss for finding it.  (I have noticed that the assembly
> code in softmmu_header.h indexes to the addend based on addr_read or
> addr_write, but adding a new member to the end of the structure
> shouldn't impact that, right?)
-------------------------
The softmmu_header.h code does assume each TLB entry has a fixed size
of (2^CPU_TLB_ENTRY_BITS) bytes. Not only the assembly code, but also
the C code assume this. So if you want to add new members into
CPUTLBEntry, add the new member at the end of the data structure, and
adjust CPU_TLB_ENTRY_BITS accordingly.

Best Regards
Tianlei Zhao

-- 
Don't look back, the lemmings are gaining on you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] CPUTLBEntry Question
  2007-06-14 13:41 ` amateur
@ 2007-06-14 14:00   ` Blue Swirl
  2007-06-15  1:47     ` amateur
  0 siblings, 1 reply; 6+ messages in thread
From: Blue Swirl @ 2007-06-14 14:00 UTC (permalink / raw)
  To: amateur, qemu-devel

On 6/14/07, amateur <tianlei.zhao@gmail.com> wrote:
> The softmmu_header.h code does assume each TLB entry has a fixed size
> of (2^CPU_TLB_ENTRY_BITS) bytes. Not only the assembly code, but also
> the C code assume this. So if you want to add new members into
> CPUTLBEntry, add the new member at the end of the data structure, and
> adjust CPU_TLB_ENTRY_BITS accordingly.

No, on Sparc32 target_ulong is 32 bits (on 32-bit host) but
target_phys_addr_t is 64 bits. That makes the structure size 20 bytes.
Everything still works.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] CPUTLBEntry Question
  2007-06-14 13:22 ` Paul Brook
@ 2007-06-14 19:31   ` Ryan Riley
  0 siblings, 0 replies; 6+ messages in thread
From: Ryan Riley @ 2007-06-14 19:31 UTC (permalink / raw)
  To: Paul Brook; +Cc: qemu-devel

On 6/14/07, Paul Brook <paul@codesourcery.com> wrote:
> > then QEMU crashes on startup.  (It also crashes if I put that blah
> > entry on the beginning instead of the end.)  I'm sure there's code
> > somewhere that must be making assumptions about the size of TLB entry,
> > but I'm at a loss for finding it.  (I have noticed that the assembly
> > code in softmmu_header.h indexes to the addend based on addr_read or
> > addr_write, but adding a new member to the end of the structure
> > shouldn't impact that, right?)
>
> Wrong. The assembly implementation assumes CPUTLBEntry is 16 bytes (or to be
> exact 1 << CPU_TLB_ENTRY_BITS).
>
> The C implementation in softmmu_header.h will work with any layout.
>
> Paul
>

That fixed/answered everything for me.  Thanks for your help, everyone.

Thanks
Ryan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Qemu-devel] CPUTLBEntry Question
  2007-06-14 14:00   ` Blue Swirl
@ 2007-06-15  1:47     ` amateur
  0 siblings, 0 replies; 6+ messages in thread
From: amateur @ 2007-06-15  1:47 UTC (permalink / raw)
  To: qemu-devel

On Thu, Jun 14, 2007 at 05:00:32PM +0300, Blue Swirl wrote:
> On 6/14/07, amateur <tianlei.zhao@gmail.com> wrote:
> >The softmmu_header.h code does assume each TLB entry has a fixed size
> >of (2^CPU_TLB_ENTRY_BITS) bytes. Not only the assembly code, but also
> >the C code assume this. So if you want to add new members into
> >CPUTLBEntry, add the new member at the end of the data structure, and
> >adjust CPU_TLB_ENTRY_BITS accordingly.
> 
> No, on Sparc32 target_ulong is 32 bits (on 32-bit host) but
> target_phys_addr_t is 64 bits. That makes the structure size 20 bytes.
> Everything still works.
-------------------------
Oh, yes. It's my fault. I confused CPU_TLB_SIZE and
CPU_TLB_ENTRY_BITS. Paul Brook is right. The C code works with any
CPUTLBEntry layout.

-- 
^[[32m题目:《村行》^[[m
^[[33m作者:王禹称(繁体“称”,换“亻”旁)(954-1001)^[[m
马穿山径菊初黄,信马悠悠野兴长。
万壑有声含晚籁,数峰无语立斜阳。
棠梨叶落胭脂色,荞麦花开白雪香。
何事吟余忽惆怅,村桥原树似吾乡。

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-06-15  1:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-13 20:25 [Qemu-devel] CPUTLBEntry Question Ryan Riley
2007-06-14 13:22 ` Paul Brook
2007-06-14 19:31   ` Ryan Riley
2007-06-14 13:41 ` amateur
2007-06-14 14:00   ` Blue Swirl
2007-06-15  1:47     ` amateur

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).