On Mon, 6 Jul 2015 12:11:01 +1000
Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
> Currently TCE tables are created once at start and their size never
> changes. We are going to change that by introducing a Dynamic DMA windows
> support where DMA configuration may change during the guest execution.
>
> This changes spapr_tce_new_table() to create an empty stub object. Only
> LIOBN is assigned by the time of creation. It still will be called once
> at the owner object (VIO or PHB) creation.
>
> This introduces an "enabled" state for TCE table objects with two
> helper functions - spapr_tce_table_enable()/spapr_tce_table_disable().
> spapr_tce_table_enable() receives TCE table parameters and allocates
> a guest view of the TCE table (in the user space or KVM).
> spapr_tce_table_disable() disposes the table.
>
> Follow up patches will disable+enable tables on reset (system reset
> or DDW reset).
>
> No visible change in behaviour is expected except the actual table
> will be reallocated every reset. We might optimize this later.
>
> The other way to implement this would be dynamically create/remove
> the TCE table QOM objects but this would make migration impossible
> as migration expects all QOM objects to exist at the receiver
> so we have to have TCE table objects created when migration begins.
>
> spapr_tce_table_do_enable() is separated from from spapr_tce_table_enable()
> as later it will be called at the sPAPRTCETable post-migration stage when
> it has all the properties set after the migration.
>
> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> ---
...
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index c1ca13d..3ddd72f 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -821,13 +821,12 @@ static int spapr_phb_dma_init_window(sPAPRPHBState
> *sphb,
> uint64_t window_size)
> {
> uint64_t bus_offset = sphb->dma32_window_start;
> - sPAPRTCETable *tcet;
> + sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
>
> - tcet = spapr_tce_new_table(DEVICE(sphb), liobn, bus_offset, page_shift,
> - window_size >> page_shift,
> - false);
> -
> - return tcet ? 0 : -1;
> + spapr_tce_table_enable(tcet, bus_offset, page_shift,
> + window_size >> page_shift,
> + false);
> + return 0;
> }
Would it be possible that this function is called with a window_size
where window_size >> page_shift results in 0?
For example triggered by a guest with a bad value for the RTAS call in
rtas_ibm_create_pe_dma_window() ?
In that case, the enablement would fail, but you'd still return 0 for
success here.
==> Maybe add a check for window_size >> page_shift == 0 and return an
error code in that case?
Thomas
