[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v1 2/4] machine: factor out enforce_aligned_dimm

From: David Hildenbrand
Subject: Re: [Qemu-devel] [PATCH v1 2/4] machine: factor out enforce_aligned_dimm into memory_device_align
Date: Wed, 20 Jun 2018 16:58:33 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.8.0

On 19.06.2018 19:06, David Hildenbrand wrote:
> On 19.06.2018 17:59, Igor Mammedov wrote:
>> On Mon, 18 Jun 2018 16:47:58 +0200
>> David Hildenbrand <address@hidden> wrote:
>>> We want to handle memory device address assignment without passing
>>> compatibility parameters ("*align").
>>> As x86 and Power use different strategies to determine an alignment and
>>> we need clean support for compat handling, let's introduce an enum on
>>> the machine class level. This is the machine configuration on how to
>>> align memory devices in guest physical memory.
>>> The three introduced types represent what is being done on x86 and Power
>>> right now.
>> commit message doesn't deliver purpose of the path,
> "We want to handle memory device address assignment without passing
> compatibility parameters ("*align")."
> So in order to do patch nr 4 without this, I would basically have to
> move the align parameter to pc_dimm_pre_plug, along with the code for
> "detecting" the alignment in e.g. pc_memory_plug. And I want to avoid
> this because ...
>> So I'm no conviced it's necessary.
>> we probably discussed it in previous revisions but could you reiterate
>> it here WHY do you need this and 3/4
> .. I want to get rid of the align parameter in the long run. Alignment
> is some memory device specific property that can be easily detected
> using a detection configuration (this patch). This approach looks much
> cleaner to me. This way we can use the same alignment strategy for all
> memory devices.
> In follow up series I want to factor out address assignment completely
> into memory_device_pre_plug(). And I also don't want to have an align
> parameter at that function. I want to avoid moving the same code around
> two times (pc.c).

It is probably best if you tell my your opinion on how address
assignment/alignment handling of pc-dimm/memory-devices is to be handled
after my rework.

My idea for the end result:

pc_dimm_pre_plug(machine, dev, errp) {
        // detect and verify slot ...
        memory_device_pre_plug(machine, dev, errp);

virtio_mem_pre_plug(machine, dev, errp) {
        memory_device_pre_plug(machine, dev, errp);

memory_device_pre_plug(machine, md, errp) {
        align = memory_device_get_align(machine, md); //handle compat
        addr = md->get_addr();
        addr = memory_device_get_free_addr(... addr, align ...)

If you want *align to remain part of the function call, then
pc_dimm_pre_plug and virtio_mem_pre_plug will have to detect the
alignment themselves (e.g. using the memory region) and either

a) pass it into memory_device_pre_plug()
b) handle what memory_device_pre_plug() would do (get_free_addr ... what
we have in pc_dimm_plug() right now)



David / dhildenb

reply via email to

[Prev in Thread] Current Thread [Next in Thread]