[Qemu-devel] for-2.1 (was Re: [PATCH] ahci: map memory via device's address space instead of address_space_memory)

[Michael S. Tsirkin]

On Thu, Jul 03, 2014 at 11:28:52AM +0300, Michael S. Tsirkin wrote:
> On Thu, Jul 03, 2014 at 04:26:27PM +0800, Le Tan wrote:
> > In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and
> > cpu_physical_memory_unmap() with dma_memory_map() and dma_memory_unmap(),
> > because ahci devices should not access memory directly but via their address
> > space. Add an AddressSpace parameter to map_page(). In order to call
> > map_page(), we should pass the AHCIState.as as the AddressSpace argument.
> > 
> > Signed-off-by: Le Tan <address@hidden>
> 
> Makes sense
> Reviewed-by: Michael S. Tsirkin <address@hidden>
> 

Stefan, Kevin, you are going to pick this one?

> > ---
> >  hw/ide/ahci.c |   21 +++++++++++----------
> >  1 file changed, 11 insertions(+), 10 deletions(-)
> > 
> > diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> > index 9bae22e..7bb0a03 100644
> > --- a/hw/ide/ahci.c
> > +++ b/hw/ide/ahci.c
> > @@ -175,17 +175,18 @@ static void ahci_trigger_irq(AHCIState *s, AHCIDevice 
> > *d,
> >      ahci_check_irq(s);
> >  }
> >  
> > -static void map_page(uint8_t **ptr, uint64_t addr, uint32_t wanted)
> > +static void map_page(AddressSpace *as, uint8_t **ptr, uint64_t addr,
> > +                     uint32_t wanted)
> >  {
> >      hwaddr len = wanted;
> >  
> >      if (*ptr) {
> > -        cpu_physical_memory_unmap(*ptr, len, 1, len);
> > +        dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
> >      }
> >  
> > -    *ptr = cpu_physical_memory_map(addr, &len, 1);
> > +    *ptr = dma_memory_map(as, addr, &len, DMA_DIRECTION_FROM_DEVICE);
> >      if (len < wanted) {
> > -        cpu_physical_memory_unmap(*ptr, len, 1, len);
> > +        dma_memory_unmap(as, *ptr, len, DMA_DIRECTION_FROM_DEVICE, len);
> >          *ptr = NULL;
> >      }
> >  }
> > @@ -198,24 +199,24 @@ static void  ahci_port_write(AHCIState *s, int port, 
> > int offset, uint32_t val)
> >      switch (offset) {
> >          case PORT_LST_ADDR:
> >              pr->lst_addr = val;
> > -            map_page(&s->dev[port].lst,
> > +            map_page(s->as, &s->dev[port].lst,
> >                       ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 
> > 1024);
> >              s->dev[port].cur_cmd = NULL;
> >              break;
> >          case PORT_LST_ADDR_HI:
> >              pr->lst_addr_hi = val;
> > -            map_page(&s->dev[port].lst,
> > +            map_page(s->as, &s->dev[port].lst,
> >                       ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 
> > 1024);
> >              s->dev[port].cur_cmd = NULL;
> >              break;
> >          case PORT_FIS_ADDR:
> >              pr->fis_addr = val;
> > -            map_page(&s->dev[port].res_fis,
> > +            map_page(s->as, &s->dev[port].res_fis,
> >                       ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 
> > 256);
> >              break;
> >          case PORT_FIS_ADDR_HI:
> >              pr->fis_addr_hi = val;
> > -            map_page(&s->dev[port].res_fis,
> > +            map_page(s->as, &s->dev[port].res_fis,
> >                       ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 
> > 256);
> >              break;
> >          case PORT_IRQ_STAT:
> > @@ -1260,9 +1261,9 @@ static int ahci_state_post_load(void *opaque, int 
> > version_id)
> >          ad = &s->dev[i];
> >          AHCIPortRegs *pr = &ad->port_regs;
> >  
> > -        map_page(&ad->lst,
> > +        map_page(s->as, &ad->lst,
> >                   ((uint64_t)pr->lst_addr_hi << 32) | pr->lst_addr, 1024);
> > -        map_page(&ad->res_fis,
> > +        map_page(s->as, &ad->res_fis,
> >                   ((uint64_t)pr->fis_addr_hi << 32) | pr->fis_addr, 256);
> >          /*
> >           * All pending i/o should be flushed out on a migrate. However,
> > -- 
> > 1.7.9.5