Am 14.04.2015 um 15:27 schrieb Igor Mammedov: > On Fri, 10 Apr 2015 16:18:06 +0800 > Chen Fan <chen.fan.f...@cn.fujitsu.com> wrote: > >> ICC bus was invented only to provide hotplug capability to >> CPU and APIC because at the time being hotplug was available only for >> BUS attached devices. >> >> Now this patch is to drop ICC bus impl, and switch to bus-less >> CPU+APIC hotplug, handling them in the same manner as pc-dimm. >> and due to APIC is bus-less, so we should reset it as CPU reset. >> >> Signed-off-by: Chen Fan <chen.fan.f...@cn.fujitsu.com> >> --- >> default-configs/i386-softmmu.mak | 1 - >> default-configs/x86_64-softmmu.mak | 1 - >> hw/cpu/Makefile.objs | 2 - >> hw/cpu/icc_bus.c | 118 >> ------------------------------------- >> hw/i386/pc.c | 19 ++---- >> hw/i386/pc_piix.c | 9 +-- >> hw/i386/pc_q35.c | 9 +-- >> hw/intc/apic.c | 6 +- >> hw/intc/apic_common.c | 5 +- >> include/hw/cpu/icc_bus.h | 82 -------------------------- >> include/hw/i386/apic_internal.h | 5 +- >> include/hw/i386/pc.h | 2 +- >> target-i386/cpu.c | 13 ++-- >> 13 files changed, 24 insertions(+), 248 deletions(-) >> delete mode 100644 hw/cpu/icc_bus.c >> delete mode 100644 include/hw/cpu/icc_bus.h >> >> diff --git a/default-configs/i386-softmmu.mak >> b/default-configs/i386-softmmu.mak >> index 6a74e00..3ac6324 100644 >> --- a/default-configs/i386-softmmu.mak >> +++ b/default-configs/i386-softmmu.mak >> @@ -39,7 +39,6 @@ CONFIG_LPC_ICH9=y >> CONFIG_PCI_Q35=y >> CONFIG_APIC=y >> CONFIG_IOAPIC=y >> -CONFIG_ICC_BUS=y >> CONFIG_PVPANIC=y >> CONFIG_MEM_HOTPLUG=y >> CONFIG_XIO3130=y >> diff --git a/default-configs/x86_64-softmmu.mak >> b/default-configs/x86_64-softmmu.mak >> index 46b87dd..5c89ea8 100644 >> --- a/default-configs/x86_64-softmmu.mak >> +++ b/default-configs/x86_64-softmmu.mak >> @@ -39,7 +39,6 @@ CONFIG_LPC_ICH9=y >> CONFIG_PCI_Q35=y >> CONFIG_APIC=y >> CONFIG_IOAPIC=y >> -CONFIG_ICC_BUS=y >> CONFIG_PVPANIC=y >> CONFIG_MEM_HOTPLUG=y >> CONFIG_XIO3130=y >> diff --git a/hw/cpu/Makefile.objs b/hw/cpu/Makefile.objs >> index 6381238..193e489 100644 >> --- a/hw/cpu/Makefile.objs >> +++ b/hw/cpu/Makefile.objs >> @@ -2,5 +2,3 @@ obj-$(CONFIG_ARM11MPCORE) += arm11mpcore.o >> obj-$(CONFIG_REALVIEW) += realview_mpcore.o >> obj-$(CONFIG_A9MPCORE) += a9mpcore.o >> obj-$(CONFIG_A15MPCORE) += a15mpcore.o >> -obj-$(CONFIG_ICC_BUS) += icc_bus.o >> - >> diff --git a/hw/cpu/icc_bus.c b/hw/cpu/icc_bus.c >> deleted file mode 100644 >> index 6646ea2..0000000 >> --- a/hw/cpu/icc_bus.c >> +++ /dev/null >> @@ -1,118 +0,0 @@ >> -/* icc_bus.c >> - * emulate x86 ICC (Interrupt Controller Communications) bus >> - * >> - * Copyright (c) 2013 Red Hat, Inc >> - * >> - * Authors: >> - * Igor Mammedov <imamm...@redhat.com> >> - * >> - * This library is free software; you can redistribute it and/or >> - * modify it under the terms of the GNU Lesser General Public >> - * License as published by the Free Software Foundation; either >> - * version 2 of the License, or (at your option) any later version. >> - * >> - * This library is distributed in the hope that it will be useful, >> - * but WITHOUT ANY WARRANTY; without even the implied warranty of >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> - * Lesser General Public License for more details. >> - * >> - * You should have received a copy of the GNU Lesser General Public >> - * License along with this library; if not, see >> <http://www.gnu.org/licenses/> >> - */ >> -#include "hw/cpu/icc_bus.h" >> -#include "hw/sysbus.h" >> - >> -/* icc-bridge implementation */ >> - >> -static const TypeInfo icc_bus_info = { >> - .name = TYPE_ICC_BUS, >> - .parent = TYPE_BUS, >> - .instance_size = sizeof(ICCBus), >> -}; >> - >> - >> -/* icc-device implementation */ >> - >> -static void icc_device_realize(DeviceState *dev, Error **errp) >> -{ >> - ICCDeviceClass *idc = ICC_DEVICE_GET_CLASS(dev); >> - >> - /* convert to QOM */ >> - if (idc->realize) { >> - idc->realize(dev, errp); >> - } >> - >> -} >> - >> -static void icc_device_class_init(ObjectClass *oc, void *data) >> -{ >> - DeviceClass *dc = DEVICE_CLASS(oc); >> - >> - dc->realize = icc_device_realize; >> - dc->bus_type = TYPE_ICC_BUS; >> -} >> - >> -static const TypeInfo icc_device_info = { >> - .name = TYPE_ICC_DEVICE, >> - .parent = TYPE_DEVICE, >> - .abstract = true, >> - .instance_size = sizeof(ICCDevice), >> - .class_size = sizeof(ICCDeviceClass), >> - .class_init = icc_device_class_init, >> -}; >> - >> - >> -/* icc-bridge implementation */ >> - >> -typedef struct ICCBridgeState { >> - /*< private >*/ >> - SysBusDevice parent_obj; >> - /*< public >*/ >> - >> - ICCBus icc_bus; >> - MemoryRegion apic_container; >> -} ICCBridgeState; >> - >> -#define ICC_BRIDGE(obj) OBJECT_CHECK(ICCBridgeState, (obj), TYPE_ICC_BRIDGE) >> - >> -static void icc_bridge_init(Object *obj) >> -{ >> - ICCBridgeState *s = ICC_BRIDGE(obj); >> - SysBusDevice *sb = SYS_BUS_DEVICE(obj); >> - >> - qbus_create_inplace(&s->icc_bus, sizeof(s->icc_bus), TYPE_ICC_BUS, >> - DEVICE(s), "icc"); >> - >> - /* Do not change order of registering regions, >> - * APIC must be first registered region, board maps it by 0 index >> - */ >> - memory_region_init(&s->apic_container, obj, "icc-apic-container", >> - APIC_SPACE_SIZE); >> - sysbus_init_mmio(sb, &s->apic_container); >> - s->icc_bus.apic_address_space = &s->apic_container; >> -} >> - >> -static void icc_bridge_class_init(ObjectClass *oc, void *data) >> -{ >> - DeviceClass *dc = DEVICE_CLASS(oc); >> - >> - set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); >> -} >> - >> -static const TypeInfo icc_bridge_info = { >> - .name = TYPE_ICC_BRIDGE, >> - .parent = TYPE_SYS_BUS_DEVICE, >> - .instance_init = icc_bridge_init, >> - .instance_size = sizeof(ICCBridgeState), >> - .class_init = icc_bridge_class_init, >> -}; >> - >> - >> -static void icc_bus_register_types(void) >> -{ >> - type_register_static(&icc_bus_info); >> - type_register_static(&icc_device_info); >> - type_register_static(&icc_bridge_info); >> -} >> - >> -type_init(icc_bus_register_types)
I think it may help to split this patch into two - one doing the APIC changes and once unused dropping the large files. >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c >> index a5e2a27..00b3ddd 100644 >> --- a/hw/i386/pc.c >> +++ b/hw/i386/pc.c >> @@ -58,7 +58,6 @@ >> #include "qemu/config-file.h" >> #include "hw/acpi/acpi.h" >> #include "hw/acpi/cpu_hotplug.h" >> -#include "hw/cpu/icc_bus.h" >> #include "hw/boards.h" >> #include "hw/pci/pci_host.h" >> #include "acpi-build.h" >> @@ -990,22 +989,17 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int >> level) >> } >> >> static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id, >> - DeviceState *icc_bridge, Error **errp) >> + Error **errp) >> { >> X86CPU *cpu = NULL; >> Error *local_err = NULL; >> >> - if (icc_bridge == NULL) { >> - error_setg(&local_err, "Invalid icc-bridge value"); >> - goto out; >> - } >> - >> cpu = cpu_x86_create(cpu_model, &local_err); >> if (local_err != NULL) { >> goto out; >> } >> >> - qdev_set_parent_bus(DEVICE(cpu), qdev_get_child_bus(icc_bridge, "icc")); >> + qdev_set_parent_bus(DEVICE(cpu), sysbus_get_default()); >> object_unref(OBJECT(cpu)); >> >> object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err); >> @@ -1024,7 +1018,6 @@ static const char *current_cpu_model; >> >> void pc_hot_add_cpu(const int64_t id, Error **errp) >> { >> - DeviceState *icc_bridge; >> int64_t apic_id = x86_cpu_apic_id_from_index(id); >> >> if (id < 0) { >> @@ -1051,12 +1044,10 @@ void pc_hot_add_cpu(const int64_t id, Error **errp) >> return; >> } >> >> - icc_bridge = DEVICE(object_resolve_path_type("icc-bridge", >> - TYPE_ICC_BRIDGE, NULL)); >> - pc_new_cpu(current_cpu_model, apic_id, icc_bridge, errp); >> + pc_new_cpu(current_cpu_model, apic_id, errp); >> } >> >> -void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge) >> +void pc_cpus_init(const char *cpu_model) >> { >> int i; >> X86CPU *cpu = NULL; >> @@ -1082,7 +1073,7 @@ void pc_cpus_init(const char *cpu_model, DeviceState >> *icc_bridge) >> >> for (i = 0; i < smp_cpus; i++) { >> cpu = pc_new_cpu(cpu_model, x86_cpu_apic_id_from_index(i), >> - icc_bridge, &error); >> + &error); >> if (error) { >> error_report_err(error); >> exit(1); >> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c >> index 1fe7bfb..6004580 100644 >> --- a/hw/i386/pc_piix.c >> +++ b/hw/i386/pc_piix.c >> @@ -39,7 +39,6 @@ >> #include "hw/kvm/clock.h" >> #include "sysemu/sysemu.h" >> #include "hw/sysbus.h" >> -#include "hw/cpu/icc_bus.h" >> #include "sysemu/arch_init.h" >> #include "sysemu/block-backend.h" >> #include "hw/i2c/smbus.h" >> @@ -98,7 +97,6 @@ static void pc_init1(MachineState *machine, >> MemoryRegion *ram_memory; >> MemoryRegion *pci_memory; >> MemoryRegion *rom_memory; >> - DeviceState *icc_bridge; >> FWCfgState *fw_cfg = NULL; >> PcGuestInfo *guest_info; >> ram_addr_t lowmem; >> @@ -143,11 +141,7 @@ static void pc_init1(MachineState *machine, >> exit(1); >> } >> >> - icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> - object_property_add_child(qdev_get_machine(), "icc-bridge", >> - OBJECT(icc_bridge), NULL); >> - >> - pc_cpus_init(machine->cpu_model, icc_bridge); >> + pc_cpus_init(machine->cpu_model); >> >> if (kvm_enabled() && kvmclock_enabled) { >> kvmclock_create(); >> @@ -230,7 +224,6 @@ static void pc_init1(MachineState *machine, >> if (pci_enabled) { >> ioapic_init_gsi(gsi_state, "i440fx"); >> } >> - qdev_init_nofail(icc_bridge); >> >> pc_register_ferr_irq(gsi[13]); >> >> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c >> index dcc17c0..3b89e6a 100644 >> --- a/hw/i386/pc_q35.c >> +++ b/hw/i386/pc_q35.c >> @@ -43,7 +43,6 @@ >> #include "hw/ide/pci.h" >> #include "hw/ide/ahci.h" >> #include "hw/usb.h" >> -#include "hw/cpu/icc_bus.h" >> #include "qemu/error-report.h" >> >> /* ICH9 AHCI has 6 ports */ >> @@ -85,7 +84,6 @@ static void pc_q35_init(MachineState *machine) >> int i; >> ICH9LPCState *ich9_lpc; >> PCIDevice *ahci; >> - DeviceState *icc_bridge; >> PcGuestInfo *guest_info; >> ram_addr_t lowmem; >> DriveInfo *hd[MAX_SATA_PORTS]; >> @@ -132,11 +130,7 @@ static void pc_q35_init(MachineState *machine) >> exit(1); >> } >> >> - icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE); >> - object_property_add_child(qdev_get_machine(), "icc-bridge", >> - OBJECT(icc_bridge), NULL); >> - >> - pc_cpus_init(machine->cpu_model, icc_bridge); >> + pc_cpus_init(machine->cpu_model); >> pc_acpi_init("q35-acpi-dsdt.aml"); >> >> kvmclock_create(); >> @@ -240,7 +234,6 @@ static void pc_q35_init(MachineState *machine) >> if (pci_enabled) { >> ioapic_init_gsi(gsi_state, "q35"); >> } >> - qdev_init_nofail(icc_bridge); >> >> pc_register_ferr_irq(gsi[13]); >> >> diff --git a/hw/intc/apic.c b/hw/intc/apic.c >> index 0f97b47..00ae0ec 100644 >> --- a/hw/intc/apic.c >> +++ b/hw/intc/apic.c >> @@ -376,7 +376,7 @@ static void apic_update_irq(APICCommonState *s) >> cpu_interrupt(cpu, CPU_INTERRUPT_POLL); >> } else if (apic_irq_pending(s) > 0) { >> cpu_interrupt(cpu, CPU_INTERRUPT_HARD); >> - } else if (!apic_accept_pic_intr(&s->busdev.qdev) || >> !pic_get_output(isa_pic)) { >> + } else if (!apic_accept_pic_intr(DEVICE(s)) || >> !pic_get_output(isa_pic)) { >> cpu_reset_interrupt(cpu, CPU_INTERRUPT_HARD); >> } >> } >> @@ -549,10 +549,10 @@ static void apic_deliver(DeviceState *dev, uint8_t >> dest, uint8_t dest_mode, >> >> static bool apic_check_pic(APICCommonState *s) >> { >> - if (!apic_accept_pic_intr(&s->busdev.qdev) || !pic_get_output(isa_pic)) >> { >> + if (!apic_accept_pic_intr(DEVICE(s)) || !pic_get_output(isa_pic)) { >> return false; >> } >> - apic_deliver_pic_intr(&s->busdev.qdev, 1); >> + apic_deliver_pic_intr(DEVICE(s), 1); >> return true; >> } >> Can you put these cast fixes in a preparatory patch please? >> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c >> index 004821a..bdc0dc7 100644 >> --- a/hw/intc/apic_common.c >> +++ b/hw/intc/apic_common.c >> @@ -436,13 +436,12 @@ static Property apic_properties_common[] = { >> >> static void apic_common_class_init(ObjectClass *klass, void *data) >> { >> - ICCDeviceClass *idc = ICC_DEVICE_CLASS(klass); >> DeviceClass *dc = DEVICE_CLASS(klass); >> >> dc->vmsd = &vmstate_apic_common; >> dc->reset = apic_reset_common; >> dc->props = apic_properties_common; >> - idc->realize = apic_common_realize; >> + dc->realize = apic_common_realize; >> /* >> * Reason: APIC and CPU need to be wired up by >> * x86_cpu_apic_create() >> @@ -452,7 +451,7 @@ static void apic_common_class_init(ObjectClass *klass, >> void *data) >> >> static const TypeInfo apic_common_type = { >> .name = TYPE_APIC_COMMON, >> - .parent = TYPE_ICC_DEVICE, >> + .parent = TYPE_DEVICE, >> .instance_size = sizeof(APICCommonState), >> .class_size = sizeof(APICCommonClass), >> .class_init = apic_common_class_init, >> diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h >> deleted file mode 100644 >> index 98a979f..0000000 >> --- a/include/hw/cpu/icc_bus.h >> +++ /dev/null >> @@ -1,82 +0,0 @@ >> -/* icc_bus.h >> - * emulate x86 ICC (Interrupt Controller Communications) bus >> - * >> - * Copyright (c) 2013 Red Hat, Inc >> - * >> - * Authors: >> - * Igor Mammedov <imamm...@redhat.com> >> - * >> - * This library is free software; you can redistribute it and/or >> - * modify it under the terms of the GNU Lesser General Public >> - * License as published by the Free Software Foundation; either >> - * version 2 of the License, or (at your option) any later version. >> - * >> - * This library is distributed in the hope that it will be useful, >> - * but WITHOUT ANY WARRANTY; without even the implied warranty of >> - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU >> - * Lesser General Public License for more details. >> - * >> - * You should have received a copy of the GNU Lesser General Public >> - * License along with this library; if not, see >> <http://www.gnu.org/licenses/> >> - */ >> -#ifndef ICC_BUS_H >> -#define ICC_BUS_H >> - >> -#include "exec/memory.h" >> -#include "hw/qdev-core.h" >> - >> -#define TYPE_ICC_BUS "icc-bus" >> - >> -#ifndef CONFIG_USER_ONLY >> - >> -/** >> - * ICCBus: >> - * >> - * ICC bus >> - */ >> -typedef struct ICCBus { >> - /*< private >*/ >> - BusState parent_obj; >> - /*< public >*/ >> - >> - MemoryRegion *apic_address_space; >> -} ICCBus; >> - >> -#define ICC_BUS(obj) OBJECT_CHECK(ICCBus, (obj), TYPE_ICC_BUS) >> - >> -/** >> - * ICCDevice: >> - * >> - * ICC device >> - */ >> -typedef struct ICCDevice { >> - /*< private >*/ >> - DeviceState qdev; >> - /*< public >*/ >> -} ICCDevice; >> - >> -/** >> - * ICCDeviceClass: >> - * @init: Initialization callback for derived classes. >> - * >> - * ICC device class >> - */ >> -typedef struct ICCDeviceClass { >> - /*< private >*/ >> - DeviceClass parent_class; >> - /*< public >*/ >> - >> - DeviceRealize realize; >> -} ICCDeviceClass; >> - >> -#define TYPE_ICC_DEVICE "icc-device" >> -#define ICC_DEVICE(obj) OBJECT_CHECK(ICCDevice, (obj), TYPE_ICC_DEVICE) >> -#define ICC_DEVICE_CLASS(klass) \ >> - OBJECT_CLASS_CHECK(ICCDeviceClass, (klass), TYPE_ICC_DEVICE) >> -#define ICC_DEVICE_GET_CLASS(obj) \ >> - OBJECT_GET_CLASS(ICCDeviceClass, (obj), TYPE_ICC_DEVICE) >> - >> -#define TYPE_ICC_BRIDGE "icc-bridge" >> - >> -#endif /* CONFIG_USER_ONLY */ >> -#endif >> diff --git a/include/hw/i386/apic_internal.h >> b/include/hw/i386/apic_internal.h >> index dc7a89d..6eeceff 100644 >> --- a/include/hw/i386/apic_internal.h >> +++ b/include/hw/i386/apic_internal.h >> @@ -21,7 +21,6 @@ >> #define QEMU_APIC_INTERNAL_H >> >> #include "exec/memory.h" >> -#include "hw/cpu/icc_bus.h" >> #include "qemu/timer.h" >> >> /* APIC Local Vector Table */ >> @@ -78,7 +77,7 @@ typedef struct APICCommonState APICCommonState; >> >> typedef struct APICCommonClass >> { >> - ICCDeviceClass parent_class; >> + DeviceClass parent_class; >> >> DeviceRealize realize; >> void (*set_base)(APICCommonState *s, uint64_t val); >> @@ -93,7 +92,7 @@ typedef struct APICCommonClass >> } APICCommonClass; >> >> struct APICCommonState { >> - ICCDevice busdev; >> + DeviceState dev; Please name parent_obj. While at it, can you please add private/public markers around the two? >> >> MemoryRegion io_memory; >> X86CPU *cpu; >> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h >> index 1b35168..7c9d044 100644 >> --- a/include/hw/i386/pc.h >> +++ b/include/hw/i386/pc.h >> @@ -163,7 +163,7 @@ extern int fd_bootchk; >> void pc_register_ferr_irq(qemu_irq irq); >> void pc_acpi_smi_interrupt(void *opaque, int irq, int level); >> >> -void pc_cpus_init(const char *cpu_model, DeviceState *icc_bridge); >> +void pc_cpus_init(const char *cpu_model); >> void pc_hot_add_cpu(const int64_t id, Error **errp); >> void pc_acpi_init(const char *default_dsdt); >> >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c >> index 03b33cf..d11b219 100644 >> --- a/target-i386/cpu.c >> +++ b/target-i386/cpu.c >> @@ -42,7 +42,6 @@ >> >> #include "sysemu/sysemu.h" >> #include "hw/qdev-properties.h" >> -#include "hw/cpu/icc_bus.h" >> #ifndef CONFIG_USER_ONLY >> #include "hw/xen/xen.h" >> #include "hw/i386/apic_internal.h" >> @@ -2634,6 +2633,14 @@ static void x86_cpu_reset(CPUState *s) >> >> xcc->parent_reset(s); >> >> + /* >> + * due to apic doesn't attach to sysbus, so when >> + * cpu reset, we should reset apic in the meanwhile. > s/.../since APIC is a bus-less device, propagate reset to it manually > >> + */ >> + if (cpu->apic_state) { >> + device_reset(DEVICE(cpu->apic_state)); >> + } >> + >> memset(env, 0, offsetof(CPUX86State, cpuid_level)); >> >> tlb_flush(s, 1); >> @@ -2757,7 +2764,6 @@ static void mce_init(X86CPU *cpu) >> #ifndef CONFIG_USER_ONLY >> static void x86_cpu_apic_create(X86CPU *cpu, Error **errp) >> { >> - DeviceState *dev = DEVICE(cpu); >> APICCommonState *apic; >> const char *apic_type = "apic"; >> >> @@ -2767,7 +2773,7 @@ static void x86_cpu_apic_create(X86CPU *cpu, Error >> **errp) >> apic_type = "xen-apic"; >> } >> >> - cpu->apic_state = qdev_try_create(qdev_get_parent_bus(dev), apic_type); >> + cpu->apic_state = DEVICE(object_new(apic_type)); >> if (cpu->apic_state == NULL) { > object_new() can't fail, hence drop this NULL check Hm, depending on why this was a "try", there's multiple options for keeping the current error handling: * check whether the type exits before object_new(), cf. CPU * use g_try_malloc0()+object_initialize() if it's a size issue That said, I like where this is going. It should help with cleaning up the recently discussed reset back-and-forth with BSP. Regards, Andreas >> error_setg(errp, "APIC device '%s' could not be created", >> apic_type); >> return; >> @@ -3009,7 +3015,6 @@ static void x86_cpu_common_class_init(ObjectClass *oc, >> void *data) >> >> xcc->parent_realize = dc->realize; >> dc->realize = x86_cpu_realizefn; >> - dc->bus_type = TYPE_ICC_BUS; >> dc->props = x86_cpu_properties; >> >> xcc->parent_reset = cc->reset; -- SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG Nürnberg)