On 01/07/14 15:49, Laszlo Ersek wrote:
>
> On 01/05/14 08:27, Qiao Nuohan wrote:
>> diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
>> index 9e47b4c..b5eaf8d 100644
>> --- a/include/sysemu/dump.h
>> +++ b/include/sysemu/dump.h
>> @@ -27,11 +27,18 @@
>> #define DUMP_DH_COMPRESSED_LZO (0x2)
>> #define DUMP_DH_COMPRESSED_SNAPPY (0x4)
>>
>> +#define PAGE_SIZE (4096)
>> #define KDUMP_SIGNATURE "KDUMP "
>> #define SIG_LEN (sizeof(KDUMP_SIGNATURE) - 1)
>> #define PHYS_BASE (0)
>> #define DUMP_LEVEL (1)
>> #define DISKDUMP_HEADER_BLOCKS (1)
>> +#define BUFSIZE_BITMAP (PAGE_SIZE)
>> +#define PFN_BUFBITMAP (CHAR_BIT * BUFSIZE_BITMAP)
>> +#define ARCH_PFN_OFFSET (0)
>> +
>> +#define paddr_to_pfn(X, page_shift) \
>> + (((unsigned long long)(X) >> (page_shift)) - ARCH_PFN_OFFSET)
>>
>> typedef struct ArchDumpInfo {
>> int d_machine; /* Architecture */
>>
>
> I think these magic constants are somewhat tied to x86, and therefore
> should be in an arch-specific file rather than a common file, but
> whoever wants to extend this to another architecture can do that.
Stressing the argument a bit more for PAGE_SIZE specifically:
- we already have TARGET_PAGE_SIZE, maybe that would be a better choice,
- PAGE_SIZE is defined *as* TARGET_PAGE_SIZE in kvm-all.c. There's no
actual conflict, but the mental conflict is bad enough.
Anyway my R-b stands.
Laszlo
