On 05/22/2014 08:11 AM, Alexander Graf wrote:
>
> On 21.05.14 16:21, Alexey Kardashevskiy wrote:
>> At the moment only 4K pages are supported by sPAPRTCETable. Since sPAPR
>> spec allows other page sizes and we are going to implement them, we need
>> page size to be configrable.
>>
>> This adds @page_shift into sPAPRTCETable and replaces SPAPR_TCE_PAGE_SHIFT
>> with it whereever it is possible.
>>
>> This removes SPAPR_TCE_PAGE_MASK as it is no longer used.
>>
>> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
>> ---
>> hw/ppc/spapr_iommu.c | 54
>> +++++++++++++++++++++++++++++---------------------
>> hw/ppc/spapr_pci.c | 1 +
>> hw/ppc/spapr_vio.c | 1 +
>> include/hw/ppc/spapr.h | 3 ++-
>> 4 files changed, 35 insertions(+), 24 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
>> index 90de3e3..e765a6d 100644
>> --- a/hw/ppc/spapr_iommu.c
>> +++ b/hw/ppc/spapr_iommu.c
>> @@ -70,12 +70,13 @@ static IOMMUTLBEntry
>> spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr)
>> if (tcet->bypass) {
>> ret.perm = IOMMU_RW;
>> - } else if ((addr >> SPAPR_TCE_PAGE_SHIFT) < tcet->nb_table) {
>> + } else if ((addr >> tcet->page_shift) < tcet->nb_table) {
>> /* Check if we are in bound */
>> - tce = tcet->table[addr >> SPAPR_TCE_PAGE_SHIFT];
>> - ret.iova = addr & ~SPAPR_TCE_PAGE_MASK;
>> - ret.translated_addr = tce & ~SPAPR_TCE_PAGE_MASK;
>> - ret.addr_mask = SPAPR_TCE_PAGE_MASK;
>> + target_ulong mask = ~((1 << tcet->page_shift) - 1);
>
> Why target_ulong? This should be u64 or hwaddr or something along those
> lines, no? Also, can the mask grow bigger than 31bits? If so you probably
> want 1ULL (below as well).
>
> In fact, we might be better off with a few more fields to tcet. Just add
> page_mask and page_size in addition to the page_shift one and use them
> instead of calculating them over and over again.
>
>> + tce = tcet->table[addr >> tcet->page_shift];
>> + ret.iova = addr & mask;
>> + ret.translated_addr = tce & mask;
>> + ret.addr_mask = ~mask;
>> ret.perm = tce;
>> }
>> trace_spapr_iommu_xlate(tcet->liobn, addr, ret.iova, ret.perm,
>> @@ -113,7 +114,7 @@ static int spapr_tce_table_realize(DeviceState *dev)
>> if (kvm_enabled()) {
>> tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
>> tcet->nb_table <<
>> - SPAPR_TCE_PAGE_SHIFT,
>> + tcet->page_shift,
>> &tcet->fd);
>> }
>> @@ -133,6 +134,7 @@ static int spapr_tce_table_realize(DeviceState *dev)
>> }
>> sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, uint32_t liobn,
>> + uint32_t page_shift,
>> uint32_t nb_table)
>> {
>> sPAPRTCETable *tcet;
>> @@ -149,6 +151,7 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState
>> *owner, uint32_t liobn,
>> tcet = SPAPR_TCE_TABLE(object_new(TYPE_SPAPR_TCE_TABLE));
>> tcet->liobn = liobn;
>> + tcet->page_shift = page_shift;
>> tcet->nb_table = nb_table;
>> object_property_add_child(OBJECT(owner), "tce-table",
>> OBJECT(tcet), NULL);
>> @@ -194,19 +197,20 @@ static target_ulong put_tce_emu(sPAPRTCETable
>> *tcet, target_ulong ioba,
>> target_ulong tce)
>> {
>> IOMMUTLBEntry entry;
>> + target_ulong mask = ~((1 << tcet->page_shift) - 1);
>> - if ((ioba >> SPAPR_TCE_PAGE_SHIFT) >= tcet->nb_table) {
>> + if ((ioba >> tcet->page_shift) >= tcet->nb_table) {
>> hcall_dprintf("spapr_vio_put_tce on out-of-bounds IOBA 0x"
>> TARGET_FMT_lx "\n", ioba);
>> return H_PARAMETER;
>> }
>> - tcet->table[ioba >> SPAPR_TCE_PAGE_SHIFT] = tce;
>> + tcet->table[ioba >> tcet->page_shift] = tce;
>> entry.target_as = &address_space_memory,
>> - entry.iova = ioba & ~SPAPR_TCE_PAGE_MASK;
>> - entry.translated_addr = tce & ~SPAPR_TCE_PAGE_MASK;
>> - entry.addr_mask = SPAPR_TCE_PAGE_MASK;
>> + entry.iova = ioba & mask;
>> + entry.translated_addr = tce & mask;
>> + entry.addr_mask = ~mask;
>> entry.perm = tce;
>> memory_region_notify_iommu(&tcet->iommu, entry);
>> @@ -226,6 +230,7 @@ static target_ulong h_put_tce_indirect(PowerPCCPU
>> *cpu,
>> target_ulong ret = H_PARAMETER;
>> sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
>> CPUState *cs = CPU(cpu);
>> + target_ulong mask;
>> if (!tcet) {
>> return H_PARAMETER;
>> @@ -235,12 +240,14 @@ static target_ulong h_put_tce_indirect(PowerPCCPU
>> *cpu,
>> return H_PARAMETER;
>> }
>> - ioba &= ~SPAPR_TCE_PAGE_MASK;
>> - tce_list &= ~SPAPR_TCE_PAGE_MASK;
>> + mask = ~((1 << tcet->page_shift) - 1);
>> + ioba &= mask;
>> +
>> + for (i = 0; i < npages; ++i, ioba += (1 << tcet->page_shift)) {
>> + target_ulong off = (tce_list & ~SPAPR_TCE_RW) +
>> + i * sizeof(target_ulong);
>> + target_ulong tce = ldq_phys(cs->as, off);
>> - for (i = 0; i < npages; ++i, ioba += SPAPR_TCE_PAGE_SIZE) {
>> - target_ulong tce = ldq_phys(cs->as, tce_list +
>> - i * sizeof(target_ulong));
>> ret = put_tce_emu(tcet, ioba, tce);
>> if (ret) {
>> break;
>> @@ -276,9 +283,9 @@ static target_ulong h_stuff_tce(PowerPCCPU *cpu,
>> sPAPREnvironment *spapr,
>> return H_PARAMETER;
>> }
>> - ioba &= ~SPAPR_TCE_PAGE_MASK;
>> + ioba &= ~((1 << tcet->page_shift) - 1);
>> - for (i = 0; i < npages; ++i, ioba += SPAPR_TCE_PAGE_SIZE) {
>> + for (i = 0; i < npages; ++i, ioba += (1 << tcet->page_shift)) {
>> ret = put_tce_emu(tcet, ioba, tce_value);
>> if (ret) {
>> break;
>> @@ -298,7 +305,7 @@ static target_ulong h_put_tce(PowerPCCPU *cpu,
>> sPAPREnvironment *spapr,
>> target_ulong ret = H_PARAMETER;
>> sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
>> - ioba &= ~(SPAPR_TCE_PAGE_SIZE - 1);
>> + ioba &= ~((1 << tcet->page_shift) - 1);
>> if (tcet) {
>> ret = put_tce_emu(tcet, ioba, tce);
>> @@ -311,13 +318,13 @@ static target_ulong h_put_tce(PowerPCCPU *cpu,
>> sPAPREnvironment *spapr,
>> static target_ulong get_tce_emu(sPAPRTCETable *tcet, target_ulong ioba,
>> target_ulong *tce)
>> {
>> - if ((ioba >> SPAPR_TCE_PAGE_SHIFT) >= tcet->nb_table) {
>> + if ((ioba >> tcet->page_shift) >= tcet->nb_table) {
>> hcall_dprintf("spapr_iommu_get_tce on out-of-bounds IOBA 0x"
>> TARGET_FMT_lx "\n", ioba);
>> return H_PARAMETER;
>> }
>> - *tce = tcet->table[ioba >> SPAPR_TCE_PAGE_SHIFT];
>> + *tce = tcet->table[ioba >> tcet->page_shift];
>> return H_SUCCESS;
>> }
>> @@ -330,8 +337,9 @@ static target_ulong h_get_tce(PowerPCCPU *cpu,
>> sPAPREnvironment *spapr,
>> target_ulong tce = 0;
>> target_ulong ret = H_PARAMETER;
>> sPAPRTCETable *tcet = spapr_tce_find_by_liobn(liobn);
>> + const target_ulong mask = ~((1 << tcet->page_shift) - 1);
>> - ioba &= ~(SPAPR_TCE_PAGE_SIZE - 1);
>> + ioba &= mask;
>> if (tcet) {
>> ret = get_tce_emu(tcet, ioba, &tce);
>> @@ -382,7 +390,7 @@ int spapr_tcet_dma_dt(void *fdt, int node_off, const
>> char *propname,
>> }
>> return spapr_dma_dt(fdt, node_off, propname,
>> - tcet->liobn, 0, tcet->nb_table <<
>> SPAPR_TCE_PAGE_SHIFT);
>> + tcet->liobn, 0, tcet->nb_table <<
>> tcet->page_shift);
>> }
>> static void spapr_tce_table_class_init(ObjectClass *klass, void *data)
>> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
>> index fdd4c07..c9850d4 100644
>> --- a/hw/ppc/spapr_pci.c
>> +++ b/hw/ppc/spapr_pci.c
>> @@ -656,6 +656,7 @@ static void spapr_phb_finish_realize(sPAPRPHBState
>> *sphb, Error **errp)
>> sPAPRTCETable *tcet;
>> tcet = spapr_tce_new_table(DEVICE(sphb), sphb->dma_liobn,
>> + SPAPR_TCE_PAGE_SHIFT,
>> 0x40000000 >> SPAPR_TCE_PAGE_SHIFT);
>> if (!tcet) {
>> error_setg(errp, "Unable to create TCE table for %s",
>> diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
>> index b84e481..d7e9e6a 100644
>> --- a/hw/ppc/spapr_vio.c
>> +++ b/hw/ppc/spapr_vio.c
>> @@ -457,6 +457,7 @@ static int spapr_vio_busdev_init(DeviceState *qdev)
>> if (pc->rtce_window_size) {
>> uint32_t liobn = SPAPR_VIO_BASE_LIOBN | dev->reg;
>> dev->tcet = spapr_tce_new_table(qdev, liobn,
>> + SPAPR_TCE_PAGE_SHIFT,
>
> I don't quite understand who defines what the TCE page size is for a given
> device. Can you try to explain this to me?
If it is default window (for PCI) or window for VIO - it is 4K. If it is a
dynamic DMA window - page size is a parameter of RTAS call which creates
the window.
--
Alexey
