Re: [Qemu-devel] [RFC PATCH v5 5/6] configure: introduce --enable-modules

[Gerd Hoffmann]

On Mi, 2013-09-11 at 09:20 +0100, Peter Maydell wrote:
> On 11 September 2013 09:01, Gerd Hoffmann <address@hidden> wrote:
> > On Mi, 2013-09-11 at 09:41 +0200, Paolo Bonzini wrote:
> >> It looks like TARGET_PAGE_SIZE is the only reason why qxl is built
> >> per-target, and in qxl_ram_set_dirty it should be enough to do
> >>
> >>     qxl_set_dirty(&qxl->vga.vram, offset, offset + 1);
> >>
> >> Would it be fine to use a generic 4096 constant everywhere else?
> >
> > Yes.  Maybe s/TARGET_PAGE_SIZE/QXL_PAGE_SIZE/ to make clear that qxl
> > operates on 4k pages.
> 
> Does this mean that the code was previously wrong for targets
> which didn't have 4K pages, or would we just have been a bit
> inefficient? I ask because ARM's TARGET_PAGE_SIZE is 1K...

Hmm.  There are three places where TARGET_PAGE_SIZE is used.

 (1) Dirtying.  The page dirtying would be just a bit inefficient
     (range is larger than it needs to be).
 (2) rom_size.  Just needs be cleaned up, is hard-coded to 8192 anyway,
     no need at all to look at the page size.
 (3) rom->num_pages field.  That one will change for arm.  The linux
     kernel qxl kms driver seems not to care at all.  Not surprising,
     it is more convenient to use the offsets in the rom to figure how
     the qxl memory layout looks like.

I still think it is better to go for a fixed page size.  Real hardware
doesn't adapt to the target page size too.  ehci for example operates on
4k pages no matter what, and arm then has to care to allocate 4 1k pages
in a row for usb xfers.

cheers,
  Gerd