Re: [Qemu-devel] [PATCH] usb-ohci: Add address masking.

["Thomas Bandelier" <thomas.bandelier@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2288 bytes --]

Hi,

Why are you writing "usb-ohci is *really* broken now." ??
Is it supposed to be working or not ?
On which status are you basing your affirmation?

Is it a good thing to completely break USB ohci in the development trunk?

Regards

Thomas




On Fri, Dec 5, 2008 at 2:52 PM, <takasi-y@ops.dti.ne.jp> wrote:

> Hi,
> About this
> > [1] It's actually the offset from the start of the first page of that
> region.
> > In practice this difference doesn't matter, and makes the implementation
> a
> > lot simpler.
> again.
>
> The patch below adds address mask(0xff) to mmio read/write funcs in
> usb-ohci.
> I know now we are deleting this kind of address mask/subtract things.
> But because of the restriction [1] above, we have to add address mask here.
>
> The mmio change potentially breaks PCI device emulations which has mmio
> region
> smaller than page size, like usb-ohci and rtl8139 (both have 256byte),
> because
> PCI memory regions are configured to be aligned to it size, but to page.
>
> usb-ohci is really broken now.
> rtl8139 is OK because it already has address mask.
> Others are not checked.
>
> I think the most important thing is eliminating dependency to base address,
> and small numbers of masks should be acceptable.
>
> Regards,
> /yoshii
>
> This patch adds address mask to mmio read/write function.
> Because its memory region is not always aligned to page.
>
> Signed-off-by: Takashi YOSHII <takasi-y@ops.dti.ne.jp>
> ---
>  hw/usb-ohci.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
> index 585f3a4..bfa7d77 100644
> --- a/hw/usb-ohci.c
> +++ b/hw/usb-ohci.c
> @@ -1366,6 +1366,7 @@ static uint32_t ohci_mem_read(void *ptr,
> target_phys_addr_t addr)
>         fprintf(stderr, "usb-ohci: Mis-aligned read\n");
>         return 0xffffffff;
>     }
> +    addr &= 0xff;
>
>     if (addr >= 0x54 && addr < 0x54 + ohci->num_ports * 4) {
>         /* HcRhPortStatus */
> @@ -1462,6 +1463,7 @@ static void ohci_mem_write(void *ptr,
> target_phys_addr_t addr, uint32_t val)
>         fprintf(stderr, "usb-ohci: Mis-aligned write\n");
>         return;
>     }
> +    addr &= 0xff;
>
>     if (addr >= 0x54 && addr < 0x54 + ohci->num_ports * 4) {
>         /* HcRhPortStatus */
> --
> 1.5.6.3
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 3130 bytes --]